clamp snprintf result before using it as fwrite length

Author: aysha-afrah26

3rdparty/minitrace/minitrace.cpp

@@ -353,6 +353,14 @@ void mtr_flush_with_state(int is_last) {
 		len = snprintf(linebuf, ARRAY_SIZE(linebuf), "%s{\"cat\":\"%s\",\"pid\":%i,\"tid\":%i,\"ts\":%" PRId64 ",\"ph\":\"%c\",\"name\":\"%s\",\"args\":{%s}%s}",
 				first_line ? "" : ",\n",
 				cat, raw->pid, raw->tid, raw->ts - time_offset, raw->ph, raw->name, arg_buf, id_buf);
+		// snprintf returns the length the line would have needed, not what it
+		// wrote, and a negative value on error, so clamp before using it as a
+		// write length.
+		if (len < 0) {
+			len = 0;
+		} else if ((size_t)len >= sizeof(linebuf)) {
+			len = (int)sizeof(linebuf) - 1;
+		}
 		fwrite(linebuf, 1, len, f);
 		first_line = 0;
 

tests/gtest_loggers.cpp

@@ -261,6 +261,33 @@ TEST_F(LoggerTest, MinitraceLogger_TransitionTypes)
   ASSERT_TRUE(std::filesystem::exists(filepath));
 }
 
+TEST_F(LoggerTest, MinitraceLogger_LongNodeName)
+{
+  // The node name goes into a 1024 byte line buffer. A name long enough to
+  // overflow it used to be written out with snprintf's would-be length, which
+  // reads past the buffer and copies stack bytes into the trace file.
+  const std::string long_name(1000, 'A');
+  const std::string xml_text = R"(
+    <root BTCPP_format="4">
+       <BehaviorTree>
+          <AlwaysSuccess name=")" +
+                               long_name + R"("/>
+       </BehaviorTree>
+    </root>)";
+
+  auto tree = factory.createTreeFromText(xml_text);
+  std::string filepath = test_dir + "/trace_long_name.json";
+
+  {
+    MinitraceLogger logger(tree, filepath.c_str());
+    tree.tickWhileRunning();
+    logger.flush();
+  }
+
+  ASSERT_TRUE(std::filesystem::exists(filepath));
+  ASSERT_LT(std::filesystem::file_size(filepath), 4096u);
+}
+
 // ============ SqliteLogger tests ============
 
 TEST_F(LoggerTest, SqliteLogger_Creation_db3)