Reject ambiguous management firmware versions without scanf

src/helpers/ManagementReporter.cpp

@@ -34,6 +34,35 @@ static unsigned parseDays(const char* text) {
   } while (*text);
   return days;
 }
+
+// MGR2 has three or four decimal version bytes, followed only by an optional
+// release/build label. Keep that label opaque for existing tagged builds.
+static bool parseManagementFirmwareVersion(const char* text, uint32_t& version) {
+  if (!text) return false;
+  if (*text == 'v') ++text;
+  uint32_t packed = 0;
+  unsigned components = 0;
+  while (true) {
+    if (*text < '0' || *text > '9') return false;
+    unsigned component = 0;
+    do {
+      const unsigned digit = *text++ - '0';
+      if (component > (255U - digit) / 10U) return false;
+      component = component * 10U + digit;
+    } while (*text >= '0' && *text <= '9');
+    packed = (packed << 8) | component;
+    ++components;
+    if (*text != '.') break;
+    if (components == 4) return false;
+    ++text;
+  }
+  if (components < 3) return false;
+  if (*text && ((*text != '-' && *text != '+') || !text[1])) return false;
+  if (components == 3) packed <<= 8;
+  version = packed;
+  return true;
+}
+
 struct ManagementReporter::Working {
   History history;
   Extrema during_report;
@@ -201,11 +230,9 @@ void ManagementReporter::snapshot() {
 #endif
   // If EndF is absent, only publish a version we can parse unambiguously.
   if (!(valid & FIRMWARE)) {
-    unsigned major = 0, minor = 0, patch = 0, pre = 0;
-    const char* version = callbacks.getFirmwareVer(); if (*version == 'v') ++version;
-    const int n = sscanf(version, "%u.%u.%u.%u", &major, &minor, &patch, &pre);
-    if (n >= 3 && major <= 255 && minor <= 255 && patch <= 255 && pre <= 255) {
-      write32(p + 28, major << 24 | minor << 16 | patch << 8 | pre); valid |= FIRMWARE;
+    uint32_t version;
+    if (parseManagementFirmwareVersion(callbacks.getFirmwareVer(), version)) {
+      write32(p + 28, version); valid |= FIRMWARE;
     }
   }
   write16(p + 76, valid);

test/fixtures/management/CommonCLI.h

@@ -29,7 +29,8 @@ public:
 };
 class CommonCLICallbacks {
 public:
-  const char* getFirmwareVer() { return "v1.17.1.5"; }
+  const char* firmware_version = "v1.17.1.5";
+  const char* getFirmwareVer() { return firmware_version; }
   const char* getRole() { return "repeater"; }
 };
 struct LocationProvider { bool isEnabled() { return false; } };

test/fixtures/management/runtime_test.cpp

@@ -58,7 +58,60 @@ struct Fixture {
     return legacy;
   }
 };
-int main() {
+static void versionSnapshot(const char* version, bool valid_version, uint32_t packed = 0) {
+  Fixture f;
+  f.callbacks.firmware_version = version;
+  f.configure(); f.advance(5 * DAY + 60);
+  assert(f.mesh.packets.size() == 1);
+  const auto* header = f.mesh.packets[0].payload;
+  assert(!memcmp(header, "MGR2", 4));
+  const bool reported_valid = (read16(header + 76) & FIRMWARE) != 0;
+  const uint32_t reported_version = read32(header + 28);
+  if (reported_valid != valid_version || reported_version != packed)
+    fprintf(stderr, "version %s: expected valid=%u packed=%08x; got valid=%u packed=%08x\n",
+        version ? version : "(null)", unsigned(valid_version), unsigned(packed),
+        unsigned(reported_valid), unsigned(reported_version));
+  assert(reported_valid == valid_version && reported_version == packed);
+}
+
+static void versionSnapshots() {
+  for (const char* version : {"1.2.3", "v1.2.3", "v001.002.003",
+      "v1.2.3-release", "v1.2.3+build_42", "v1.2.3-rc.1+build_42"})
+    versionSnapshot(version, true, 0x01020300);
+  for (const char* version : {"1.2.3.4", "v1.2.3.004", "v1.2.3.4-preview1",
+      "v1.2.3.4+build_42", "v1.2.3.4-label_with_underscores"})
+    versionSnapshot(version, true, 0x01020304);
+  versionSnapshot("v1.18.0", true, 0x01120000); // Direct-build example default.
+  versionSnapshot("v1.17.1.9-halo-keymind-cascade-dev-5f4d242f", true, 0x01110109);
+  versionSnapshot("v1.17.1.9-halo-keymind-cascade-dev.42-observer-dev-5f4d242f", true, 0x01110109);
+  versionSnapshot("v1.18.0.42-5f4d242f", true, 0x0112002a); // Three-part base + build number.
+  versionSnapshot("255.255.255.255", true, UINT32_MAX);
+  versionSnapshot("0.0.0", true);
+  versionSnapshot("v0.0.0.0+build_0", true);
+  for (const char* version : {"", "v", "1", "1.2", "1..3", "1.2.", "1.2.3.",
+      "1.2.3.4.5", "1.2.3.xyz", "1.2.3garbage", "1.2.3.4garbage", "1.2.3 arbitrary text",
+      "1.2.3-", "1.2.3+", "1.2.3.4-", "1.2.3.4+", "release-v1.2.3",
+      " 1.2.3", "1. 2.3", "1.2.3 ", "1.2.3. 4", "+1.2.3", "1.+2.3", "1.2.-3",
+      "1.2.3.+4", "1.2.3.-4", "256.0.0", "0.256.0", "0.0.256", "0.0.0.256",
+      "v4294967297.2.3", "v1.2.3.4294967297", "v-4294967295.2.3", "dev-5f4d242f"})
+    versionSnapshot(version, false);
+  const std::string huge_component(4096, '9');
+  versionSnapshot(("v" + huge_component + ".2.3").c_str(), false);
+  versionSnapshot(("v1.2.3." + huge_component).c_str(), false);
+  versionSnapshot(nullptr, false);
+}
+
+int main(int argc, char** argv) {
+  if (argc == 2 && !strcmp(argv[1], "versions")) {
+    versionSnapshots();
+    puts("PASS production MGR2 firmware version snapshots");
+    return 0;
+  }
+  if (argc == 2 && !strcmp(argv[1], "version-malformed")) {
+    versionSnapshot("v1.2.3garbage", false);
+    return 0;
+  }
+  assert(argc == 1);
   {
     Fixture f;
     assert(f.cmd("get mgmt").find("direct=5d") != std::string::npos);

test/test_management_report.py

@@ -18,6 +18,18 @@ spec = importlib.util.spec_from_file_location("management", ROOT / "tools/manage
 report = importlib.util.module_from_spec(spec)
 spec.loader.exec_module(report)
 
+# Audited old snapshot fallback: the real libc conversion accepts a malformed
+# numeric prefix as a valid-looking MGR2 version. Keep a negative runtime
+# control so the snapshot regression proves it detects that bug.
+LEGACY_VERSION_FALLBACK = r'''if (!(valid & FIRMWARE)) {
+    unsigned major = 0, minor = 0, patch = 0, pre = 0;
+    const char* version = callbacks.getFirmwareVer(); if (*version == 'v') ++version;
+    const int n = sscanf(version, "%u.%u.%u.%u", &major, &minor, &patch, &pre);
+    if (n >= 3 && major <= 255 && minor <= 255 && patch <= 255 && pre <= 255) {
+      write32(p + 28, major << 24 | minor << 16 | patch << 8 | pre); valid |= FIRMWARE;
+    }
+  }'''
+
 
 class ManagementTests(unittest.TestCase):
     @classmethod
@@ -76,6 +88,17 @@ class ManagementTests(unittest.TestCase):
         runtime_cmd += [str(cls.crypto / p) for p in sources] + ["-o", str(runtime)]
         subprocess.run(runtime_cmd, check=True, capture_output=True, text=True)
         cls.runtime = runtime
+        runtime_source = Path(cls.work.name) / "ManagementReporter.cpp"
+        runtime_text = runtime_source.read_text(encoding="ascii")
+        fallback = extract_braced(runtime_text, "if (!(valid & FIRMWARE))")
+        legacy_source = Path(cls.work.name) / "ManagementReporter-legacy.cpp"
+        legacy_source.write_text(runtime_text.replace(fallback, LEGACY_VERSION_FALLBACK, 1),
+                                 encoding="ascii")
+        cls.legacy_runtime = Path(cls.work.name) / "runtime-legacy-version"
+        legacy_cmd = list(runtime_cmd)
+        legacy_cmd[legacy_cmd.index(str(runtime_source))] = str(legacy_source)
+        legacy_cmd[-1] = str(cls.legacy_runtime)
+        subprocess.run(legacy_cmd, check=True, capture_output=True, text=True)
 
     @classmethod
     def tearDownClass(cls):
@@ -437,6 +460,20 @@ process.stdin.on('end', async () => {
     def test_runtime_storage_scheduling_rollover_and_failures(self):
         subprocess.run([str(self.runtime)], check=True)
 
+    def test_production_snapshot_versions_are_bounded_and_release_compatible(self):
+        result = subprocess.run([str(self.runtime), "versions"], capture_output=True,
+                                text=True, timeout=30)
+        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        self.assertIn("PASS production MGR2 firmware version snapshots", result.stdout)
+
+    def test_previous_snapshot_parser_reports_malformed_input_as_a_valid_version(self):
+        result = subprocess.run([str(self.legacy_runtime), "version-malformed"], capture_output=True,
+                                text=True, timeout=30)
+        self.assertNotEqual(result.returncode, 0)
+        self.assertIn("version v1.2.3garbage: expected valid=0", result.stderr)
+        self.assertIn("got valid=1", result.stderr)
+        self.assertIn("Assertion", result.stderr)
+
     def test_actual_legacy_route_adoption_persistence_and_precedence(self):
         source = (ROOT / "src/helpers/CommonCLI_Management.cpp").read_text(encoding="utf-8")
         state = source[source.index("static constexpr char DATA_ROUTE_FILE[]"):