clamp snprintf result before using it as fwrite length (#1185)

Backport of hrydgard/minitrace#49 (merged upstream as 679ac44) into the
vendored copy. mtr_flush_with_state passed snprintf's would-be length to
fwrite, so an event name longer than the 1024 byte line buffer made
fwrite read past linebuf and copy stack bytes into the trace file.
MinitraceLogger reaches this with any node name from the tree XML.

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

@@ -360,6 +360,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)