Просмотр исходного кода

fix: improve error handling of snprintf() in send_thread_stop_status() (#4234)

Prevent `MAX_PACKET_SIZE - len` from overflowing.
liang.he 8 месяцев назад
Родитель
Сommit
8f3961026e
1 измененных файлов с 57 добавлено и 24 удалено
  1. 57 24
      core/iwasm/libraries/debug-engine/handler.c

+ 57 - 24
core/iwasm/libraries/debug-engine/handler.c

@@ -383,7 +383,7 @@ send_thread_stop_status(WASMGDBServer *server, uint32 status, korp_tid tid)
 
     if (status == 0) {
         os_mutex_lock(&tmpbuf_lock);
-        snprintf(tmpbuf, MAX_PACKET_SIZE, "W%02" PRIx32, status);
+        (void)snprintf(tmpbuf, MAX_PACKET_SIZE, "W%02" PRIx32, status);
         write_packet(server, tmpbuf);
         os_mutex_unlock(&tmpbuf_lock);
         return;
@@ -399,18 +399,38 @@ send_thread_stop_status(WASMGDBServer *server, uint32 status, korp_tid tid)
 
     os_mutex_lock(&tmpbuf_lock);
     // TODO: how name a wasm thread?
-    len += snprintf(tmpbuf, MAX_PACKET_SIZE,
-                    "T%02" PRIx32 "thread:%" PRIx64 ";name:%s;", gdb_status,
-                    (uint64)(uintptr_t)tid, "nobody");
+    len = snprintf(tmpbuf, MAX_PACKET_SIZE,
+                   "T%02" PRIx32 "thread:%" PRIx64 ";name:%s;", gdb_status,
+                   (uint64)(uintptr_t)tid, "nobody");
+    if (len < 0 || len >= MAX_PACKET_SIZE) {
+        os_mutex_unlock(&tmpbuf_lock);
+        return;
+    }
+
     if (tids_count > 0) {
-        len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "threads:");
+        int n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "threads:");
+        if (n < 0 || n >= MAX_PACKET_SIZE - len) {
+            os_mutex_unlock(&tmpbuf_lock);
+            return;
+        }
+
+        len += n;
         while (i < tids_count) {
-            if (i == tids_count - 1)
-                len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
-                                "%" PRIx64 ";", (uint64)(uintptr_t)tids[i]);
-            else
-                len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
-                                "%" PRIx64 ",", (uint64)(uintptr_t)tids[i]);
+            if (i == tids_count - 1) {
+                n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
+                             "%" PRIx64 ";", (uint64)(uintptr_t)tids[i]);
+            }
+            else {
+                n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
+                             "%" PRIx64 ",", (uint64)(uintptr_t)tids[i]);
+            }
+
+            if (n < 0 || n >= MAX_PACKET_SIZE - len) {
+                os_mutex_unlock(&tmpbuf_lock);
+                return;
+            }
+
+            len += n;
             i++;
         }
     }
@@ -427,32 +447,45 @@ send_thread_stop_status(WASMGDBServer *server, uint32 status, korp_tid tid)
         /* When exception occurs, use reason:exception so the description can be
          * correctly processed by LLDB */
         uint32 exception_len = strlen(exception);
-        len +=
+        int n =
             snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
                      "thread-pcs:%" PRIx64 ";00:%s;reason:%s;description:", pc,
                      pc_string, "exception");
+        if (n < 0 || n >= MAX_PACKET_SIZE - len) {
+            os_mutex_unlock(&tmpbuf_lock);
+            return;
+        }
+
+        len += n;
         /* The description should be encoded as HEX */
         for (i = 0; i < exception_len; i++) {
-            len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "%02x",
-                            exception[i]);
+            n = snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, "%02x",
+                         exception[i]);
+            if (n < 0 || n >= MAX_PACKET_SIZE - len) {
+                os_mutex_unlock(&tmpbuf_lock);
+                return;
+            }
+
+            len += n;
         }
-        len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, ";");
+
+        (void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len, ";");
     }
     else {
         if (status == WAMR_SIG_TRAP) {
-            len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
-                            "thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
-                            pc_string, "breakpoint");
+            (void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
+                           "thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
+                           pc_string, "breakpoint");
         }
         else if (status == WAMR_SIG_SINGSTEP) {
-            len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
-                            "thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
-                            pc_string, "trace");
+            (void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
+                           "thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
+                           pc_string, "trace");
         }
         else { /* status > 0 (== 0 is checked at the function beginning) */
-            len += snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
-                            "thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
-                            pc_string, "signal");
+            (void)snprintf(tmpbuf + len, MAX_PACKET_SIZE - len,
+                           "thread-pcs:%" PRIx64 ";00:%s;reason:%s;", pc,
+                           pc_string, "signal");
         }
     }
     write_packet(server, tmpbuf);