Skip to content

Commit 8f39610

Browse files
authored
fix: improve error handling of snprintf() in send_thread_stop_status() (#4234)
Prevent `MAX_PACKET_SIZE - len` from overflowing.
1 parent 908838a commit 8f39610

File tree

1 file changed

+57
-24
lines changed

1 file changed

+57
-24
lines changed

core/iwasm/libraries/debug-engine/handler.c

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ send_thread_stop_status(WASMGDBServer *server, uint32 status, korp_tid tid)
383383

384384
if (status == 0) {
385385
os_mutex_lock(&tmpbuf_lock);
386-
snprintf(tmpbuf, MAX_PACKET_SIZE, "W%02" PRIx32, status);
386+
(void)snprintf(tmpbuf, MAX_PACKET_SIZE, "W%02" PRIx32, status);
387387
write_packet(server, tmpbuf);
388388
os_mutex_unlock(&tmpbuf_lock);
389389
return;
@@ -399,18 +399,38 @@ send_thread_stop_status(WASMGDBServer *server, uint32 status, korp_tid tid)
399399

400400
os_mutex_lock(&tmpbuf_lock);
401401
// TODO: how name a wasm thread?
402-
len += snprintf(tmpbuf, MAX_PACKET_SIZE,
403-
"T%02" PRIx32 "thread:%" PRIx64 ";name:%s;", gdb_status,
404-
(uint64)(uintptr_t)tid, "nobody");
402+
len = snprintf(tmpbuf, MAX_PACKET_SIZE,
403+
"T%02" PRIx32 "thread:%" PRIx64 ";name:%s;", gdb_status,
404+
(uint64)(uintptr_t)tid, "nobody");
405+
if (len < 0 || len >= MAX_PACKET_SIZE) {
406+
os_mutex_unlock(&tmpbuf_lock);
407+
return;
408+
}
409+
405410
if (tids_count > 0) {
406-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "threads:");
411+
int n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "threads:");
412+
if (n < 0 || n >= MAX_PACKET_SIZE - len) {
413+
os_mutex_unlock(&tmpbuf_lock);
414+
return;
415+
}
416+
417+
len += n;
407418
while (i < tids_count) {
408-
if (i == tids_count - 1)
409-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
410-
"%" PRIx64 ";", (uint64)(uintptr_t)tids[i]);
411-
else
412-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
413-
"%" PRIx64 ",", (uint64)(uintptr_t)tids[i]);
419+
if (i == tids_count - 1) {
420+
n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
421+
"%" PRIx64 ";", (uint64)(uintptr_t)tids[i]);
422+
}
423+
else {
424+
n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
425+
"%" PRIx64 ",", (uint64)(uintptr_t)tids[i]);
426+
}
427+
428+
if (n < 0 || n >= MAX_PACKET_SIZE - len) {
429+
os_mutex_unlock(&tmpbuf_lock);
430+
return;
431+
}
432+
433+
len += n;
414434
i++;
415435
}
416436
}
@@ -427,32 +447,45 @@ send_thread_stop_status(WASMGDBServer *server, uint32 status, korp_tid tid)
427447
/* When exception occurs, use reason:exception so the description can be
428448
* correctly processed by LLDB */
429449
uint32 exception_len = strlen(exception);
430-
len +=
450+
int n =
431451
snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
432452
"thread-pcs:%" PRIx64 ";00:%s;reason:%s;description:", pc,
433453
pc_string, "exception");
454+
if (n < 0 || n >= MAX_PACKET_SIZE - len) {
455+
os_mutex_unlock(&tmpbuf_lock);
456+
return;
457+
}
458+
459+
len += n;
434460
/* The description should be encoded as HEX */
435461
for (i = 0; i < exception_len; i++) {
436-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "%02x",
437-
exception[i]);
462+
n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "%02x",
463+
exception[i]);
464+
if (n < 0 || n >= MAX_PACKET_SIZE - len) {
465+
os_mutex_unlock(&tmpbuf_lock);
466+
return;
467+
}
468+
469+
len += n;
438470
}
439-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, ";");
471+
472+
(void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, ";");
440473
}
441474
else {
442475
if (status == WAMR_SIG_TRAP) {
443-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
444-
"thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
445-
pc_string, "breakpoint");
476+
(void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
477+
"thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
478+
pc_string, "breakpoint");
446479
}
447480
else if (status == WAMR_SIG_SINGSTEP) {
448-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
449-
"thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
450-
pc_string, "trace");
481+
(void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
482+
"thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
483+
pc_string, "trace");
451484
}
452485
else { /* status > 0 (== 0 is checked at the function beginning) */
453-
len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
454-
"thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
455-
pc_string, "signal");
486+
(void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
487+
"thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
488+
pc_string, "signal");
456489
}
457490
}
458491
write_packet(server, tmpbuf);

0 commit comments

Comments
 (0)