From f6c00be3e00f25fec7b2168d976c0543a4b31b9c Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 12:54:36 -0700 Subject: [PATCH] Bound region name exports for small destinations --- .github/workflows/run-unit-tests.yml | 1 + src/helpers/RegionMap.cpp | 3 +- src/helpers/RegionMap.h | 2 + test/fixtures/region_name_export.cpp | 313 +++++++++++++++++++++++++++ test/test_region_name_export.py | 101 +++++++++ 5 files changed, 419 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/region_name_export.cpp create mode 100644 test/test_region_name_export.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 3bc4545c6..ba095513e 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -185,6 +185,7 @@ jobs: run: | python3 -B test/test_protocol_parser_bounds.py -v python3 -B test/test_region_map_transactions.py -v + python3 -B test/test_region_name_export.py -v python3 -B test/test_cli_error_bounds.py -v python3 -B test/test_common_cli_argument_bounds.py -v python3 -B test/test_flood_retry_bucket_cli.py -v diff --git a/src/helpers/RegionMap.cpp b/src/helpers/RegionMap.cpp index 3c7521cf4..e5eb8baa8 100644 --- a/src/helpers/RegionMap.cpp +++ b/src/helpers/RegionMap.cpp @@ -486,11 +486,12 @@ size_t RegionMap::exportTo(char *dest, size_t max_len) const { } int RegionMap::exportNamesTo(char *dest, int max_len, uint8_t mask, bool invert) { + if (!dest || max_len <= 0) return 0; char *dp = dest; // Check wildcard region bool wildcard_matches = invert ? (wildcard.flags & mask) : !(wildcard.flags & mask); - if (wildcard_matches) { + if (wildcard_matches && max_len >= 2) { *dp++ = '*'; *dp++ = ','; } diff --git a/src/helpers/RegionMap.h b/src/helpers/RegionMap.h index de44f8bd0..d45d0074d 100644 --- a/src/helpers/RegionMap.h +++ b/src/helpers/RegionMap.h @@ -54,6 +54,8 @@ public: int getCount() const { return num_regions; } const RegionEntry* getByIdx(int i) const { return ®ions[i]; } const RegionEntry* getRoot() const { return &wildcard; } + // Capacity includes the terminator. Null/nonpositive capacity writes nothing. + // Every non-null destination with positive capacity is null-terminated. int exportNamesTo(char *dest, int max_len, uint8_t mask, bool invert = false); int getTransportKeysFor(const RegionEntry& src, TransportKey dest[], int max_num); diff --git a/test/fixtures/region_name_export.cpp b/test/fixtures/region_name_export.cpp new file mode 100644 index 000000000..1c84a497b --- /dev/null +++ b/test/fixtures/region_name_export.cpp @@ -0,0 +1,313 @@ +// Exercise the actual RegionMap declarations and method bodies, including the +// actual Repeater/Room getLocalScopes wrappers through minimal role adapters. +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "region_under_test.inc" + +// Frozen original exporter from commit 3a3323c4d594dd766dea25caaad06bb5225a8ffa, +// RegionMap.cpp SHA256 0f3dd22e2c228061334bcf95416cf40ecdd01310f99a011785a0837e43334900. +// The adapter snapshots real public region state. The method below is unchanged +// and is used only for >=2-byte legacy output compatibility, never for safety. +namespace legacy { +class RegionMap { + uint16_t num_regions; + RegionEntry regions[MAX_REGION_ENTRIES]; + RegionEntry wildcard; +public: + explicit RegionMap(::RegionMap& source) + : num_regions(source.getCount()), wildcard(source.getWildcard()) { + for (int i = 0; i < num_regions; ++i) regions[i] = *source.getByIdx(i); + } + int exportNamesTo(char *dest, int max_len, uint8_t mask, bool invert = false); +}; + +int RegionMap::exportNamesTo(char *dest, int max_len, uint8_t mask, bool invert) { + char *dp = dest; + + // Check wildcard region + bool wildcard_matches = invert ? (wildcard.flags & mask) : !(wildcard.flags & mask); + if (wildcard_matches) { + *dp++ = '*'; + *dp++ = ','; + } + + for (int i = 0; i < num_regions; i++) { + auto region = ®ions[i]; + + // Check if region matches the filter criteria + bool region_matches = invert ? (region->flags & mask) : !(region->flags & mask); + + if (region_matches) { + int len = strlen(skip_hash(region->name)); + if ((dp - dest) + len + 2 < max_len) { // only append if name will fit + memcpy(dp, skip_hash(region->name), len); + dp += len; + *dp++ = ','; + } + } + } + + if (dp > dest) { dp--; } // don't include trailing comma + + *dp = 0; // set null terminator + return dp - dest; // return length +} +} // namespace legacy + +static void require(bool condition, const char* message) { + if (!condition) { + std::fprintf(stderr, "FAIL: %s\n", message); + std::exit(1); + } +} + +static void populate(RegionMap& map, bool dense) { + if (dense) { + for (int i = 0; i < MAX_REGION_ENTRIES; ++i) { + char name[31]; + std::snprintf(name, sizeof(name), "#Place%02d", i); + RegionEntry* entry = map.putRegion(name, 0); + require(entry != nullptr, "populate full real RegionMap"); + entry->flags = 0; + } + return; + } + const char* names[] = {"#Alpine", "$Secret", "B", "#Direct", "Both", + "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234"}; + const uint8_t flags[] = {0, 0, REGION_DENY_FLOOD, REGION_DENY_DIRECT, + REGION_DENY_FLOOD | REGION_DENY_DIRECT, 0}; + for (size_t i = 0; i < sizeof(names) / sizeof(names[0]); ++i) { + RegionEntry* entry = map.putRegion(names[i], 0); + require(entry != nullptr, "populate region names and filter policies"); + entry->flags = flags[i]; + } +} + +static std::vector eligible(RegionMap& map, uint8_t mask, bool invert) { + std::vector names; + const auto matches = [=](uint8_t flags) { + return invert ? (flags & mask) != 0 : (flags & mask) == 0; + }; + if (matches(map.getWildcard().flags)) names.emplace_back("*"); + for (int i = 0; i < map.getCount(); ++i) { + const RegionEntry* entry = map.getByIdx(i); + if (matches(entry->flags)) names.emplace_back(RegionNameUtils::canonical(entry->name)); + } + return names; +} + +static std::string joined(const std::vector& names) { + std::string result; + for (const auto& name : names) { + if (!result.empty()) result += ','; + result += name; + } + return result; +} + +static void validate_output(const char* destination, int capacity, int count, + const std::vector& names) { + require(count >= 0 && count < capacity, "returned length is inside destination capacity"); + require(destination[count] == 0, "returned length points to null terminator"); + require(std::strlen(destination) == static_cast(count), + "returned length is exactly the produced string length"); + const std::string actual(destination); + require(actual.empty() || (actual.front() != ',' && actual.back() != ','), + "no leading or trailing comma"); + size_t allowed = 0; + for (size_t begin = 0; begin < actual.size();) { + const size_t end = actual.find(',', begin); + const std::string token = actual.substr(begin, end == std::string::npos ? end : end - begin); + require(!token.empty(), "no empty region token"); + while (allowed < names.size() && names[allowed] != token) ++allowed; + require(allowed < names.size(), "only complete matching names in original order"); + ++allowed; + if (end == std::string::npos) break; + begin = end + 1; + } + if (joined(names).size() + 3 < static_cast(capacity)) { + require(actual == joined(names), "ample destination includes every matching name"); + } +} + +static void guarded_export(RegionMap& map, int capacity, uint8_t mask, bool invert) { + constexpr size_t guard = 32; + constexpr unsigned char sentinel = 0xA5; + const size_t accessible = capacity > 0 ? static_cast(capacity) : 0; + std::vector storage(guard + accessible + guard, sentinel); + char* destination = reinterpret_cast(storage.data() + guard); + const int count = map.exportNamesTo(destination, capacity, mask, invert); + require(std::all_of(storage.begin(), storage.begin() + guard, + [](unsigned char c) { return c == sentinel; }), + "export preserved prefix guard"); + require(std::all_of(storage.begin() + guard + accessible, storage.end(), + [](unsigned char c) { return c == sentinel; }), + "export wrote no byte beyond claimed destination capacity"); + if (capacity <= 0) { + require(count == 0, "nonpositive destination returns zero without writing"); + } else { + validate_output(destination, capacity, count, eligible(map, mask, invert)); + if (capacity == 1) require(destination[0] == 0, "one-byte destination is empty"); + if (capacity >= 2) { + legacy::RegionMap original(map); + std::vector baseline(accessible, sentinel); + const int old_count = original.exportNamesTo( + reinterpret_cast(baseline.data()), capacity, mask, invert); + require(count == old_count + && std::equal(baseline.begin(), baseline.end(), storage.begin() + guard), + "all >=2-byte results and destination bytes match frozen original exporter"); + } + } +} + +template +static void guarded_wrapper(RegionMap& map, size_t capacity) { + constexpr size_t guard = 32; + constexpr unsigned char sentinel = 0xA5; + std::vector storage(guard + capacity + guard, sentinel); + char* destination = reinterpret_cast(storage.data() + guard); + Role role(map); + role.getLocalScopes(destination, capacity); + require(std::all_of(storage.begin(), storage.begin() + guard, + [](unsigned char c) { return c == sentinel; }), + "role wrapper preserved prefix guard"); + require(std::all_of(storage.begin() + guard + capacity, storage.end(), + [](unsigned char c) { return c == sentinel; }), + "role wrapper wrote no byte beyond claimed destination capacity"); + if (capacity) { + const void* end = std::memchr(destination, 0, capacity); + require(end != nullptr, "role wrapper terminates inside supplied buffer"); + validate_output(destination, static_cast(capacity), + static_cast(end) - destination, + eligible(map, REGION_DENY_FLOOD, false)); + } +} + +static void healthy() { + TransportKeyStore store; + RegionMap map(store); + populate(map, false); + for (int capacity : {96, 159, 160}) { + char destination[160]; + const char* golden = "*,Alpine,$Secret,Direct,ABCDEFGHIJKLMNOPQRSTUVWXYZ1234"; + const int count = map.exportNamesTo(destination, capacity, REGION_DENY_FLOOD); + require(std::strcmp(destination, golden) == 0 && count == static_cast(std::strlen(golden)), + "healthy production-size output unchanged"); + guarded_wrapper(map, capacity); + guarded_wrapper(map, capacity); + } + RegionMap full(store); + populate(full, true); + const char* golden96 = "*,Place00,Place01,Place02,Place03,Place04,Place05,Place06,Place07,Place08,Place09,Place10"; + const char* golden159 = "*,Place00,Place01,Place02,Place03,Place04,Place05,Place06,Place07,Place08,Place09,Place10,Place11,Place12,Place13,Place14,Place15,Place16,Place17,Place18"; + for (int capacity : {96, 159, 160}) { + char destination[160]; + const char* golden = capacity == 96 ? golden96 : golden159; + const int count = full.exportNamesTo(destination, capacity, REGION_DENY_FLOOD); + require(std::strcmp(destination, golden) == 0 && count == static_cast(std::strlen(golden)), + "full-map production-size truncation unchanged"); + } +} + +static void invalid() { + TransportKeyStore store; + RegionMap map(store); + populate(map, false); + for (int capacity : {INT_MIN, -4, -1, 0, 1, 2, 160, INT_MAX}) { + require(map.exportNamesTo(nullptr, capacity, REGION_DENY_FLOOD) == 0, + "null destination returns zero without access"); + } + for (int capacity : {INT_MIN, -4, -1, 0}) { + guarded_export(map, capacity, REGION_DENY_FLOOD, false); + } + repeater::MyMesh repeater(map); + room::MyMesh room(map); + for (size_t capacity : {size_t(0), size_t(1), size_t(160)}) { + repeater.getLocalScopes(nullptr, capacity); + room.getLocalScopes(nullptr, capacity); + } +} + +static void matrix() { + TransportKeyStore store; + size_t cases = 0; + for (int shape = 0; shape < 3; ++shape) { + RegionMap map(store); + if (shape) populate(map, shape == 2); + for (uint8_t wildcard_flags = 0; wildcard_flags < 4; ++wildcard_flags) { + map.getWildcard().flags = wildcard_flags; + for (uint8_t mask = 0; mask < 4; ++mask) { + for (bool invert : {false, true}) { + for (int capacity = -1; capacity <= 320; ++capacity) { + guarded_export(map, capacity, mask, invert); + ++cases; + } + } + } + } + } + std::printf("PASS: %zu direct capacity/filter combinations\n", cases); +} + +static void wrappers() { + TransportKeyStore store; + for (int shape = 0; shape < 3; ++shape) { + RegionMap map(store); + if (shape) populate(map, shape == 2); + for (uint8_t wildcard_flags = 0; wildcard_flags < 4; ++wildcard_flags) { + map.getWildcard().flags = wildcard_flags; + for (size_t capacity = 0; capacity <= 320; ++capacity) { + guarded_wrapper(map, capacity); + guarded_wrapper(map, capacity); + } + } + } +} + +template +static void one_byte() { + TransportKeyStore store; + RegionMap map(store); + Role role(map); + // An exact one-byte allocation makes an out-of-capacity intermediate comma + // store observable to ASan even if the eventual terminator is otherwise valid. + std::unique_ptr destination(new char[1]); + destination[0] = '!'; + role.getLocalScopes(destination.get(), 1); + require(destination[0] == 0, "one-byte role wrapper produces an empty string"); +} + +static void direct_one_byte() { + TransportKeyStore store; + RegionMap map(store); + std::unique_ptr destination(new char[1]); + destination[0] = '!'; + require(map.exportNamesTo(destination.get(), 1, REGION_DENY_FLOOD) == 0, + "one-byte direct exporter returns zero"); + require(destination[0] == 0, "one-byte direct exporter produces an empty string"); +} + +int main(int argc, char** argv) { + require(argc == 2, "one scenario argument required"); + const std::string scenario(argv[1]); + if (scenario == "healthy") healthy(); + else if (scenario == "invalid") invalid(); + else if (scenario == "matrix") matrix(); + else if (scenario == "wrappers") wrappers(); + else if (scenario == "direct-one-byte") direct_one_byte(); + else if (scenario == "repeater-one-byte") one_byte(); + else if (scenario == "room-one-byte") one_byte(); + else require(false, "known scenario"); + std::printf("PASS: %s\n", scenario.c_str()); + return 0; +} diff --git a/test/test_region_name_export.py b/test/test_region_name_export.py new file mode 100644 index 000000000..4f9980261 --- /dev/null +++ b/test/test_region_name_export.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +"""Bounded RegionMap name export through the real role-facing wrappers.""" + +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +from cpp_source import body + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / "test/fixtures/region_name_export.cpp" + + +def implementation(): + source = (ROOT / "src/helpers/RegionMap.cpp").read_text(encoding="ascii") + signatures = ( + "static const char* skip_hash(", "RegionMap::RegionMap(", + "bool RegionMap::is_name_char(", "RegionEntry* RegionMap::putRegion(", + "RegionEntry* RegionMap::findByName(", "RegionEntry* RegionMap::findById(", + "bool RegionMap::wouldCreateCycle(", "int RegionMap::exportNamesTo(", + ) + generated = "\n".join(body(source, signature) for signature in signatures) + text_helpers = (ROOT / "src/helpers/TxtDataHelpers.cpp").read_text(encoding="ascii") + generated += "\n" + body(text_helpers, "void StrHelper::strncpy(") + for role, directory in (("repeater", "simple_repeater"), + ("room", "simple_room_server")): + role_source = (ROOT / "examples" / directory / "MyMesh.cpp").read_text(encoding="ascii") + # Keep the original production wrapper body and signature unchanged. + # A namespace gives each minimal role adapter its original MyMesh name. + generated += "\nnamespace " + role + " {\n" + generated += "class MyMesh {\npublic:\nRegionMap& region_map;\n" + generated += "explicit MyMesh(RegionMap& map) : region_map(map) {}\n" + generated += "void getLocalScopes(char* buf, size_t len);\n};\n" + generated += body(role_source, "void MyMesh::getLocalScopes(") + "\n}\n" + return generated + + +class RegionNameExportTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + compiler = shutil.which("g++") or shutil.which("clang++") + if compiler is None: + raise AssertionError("a native C++ compiler is required") + cls.temporary = tempfile.TemporaryDirectory(prefix="meshcore-region-name-export-") + work = Path(cls.temporary.name) + (work / "region_under_test.inc").write_text(implementation(), encoding="ascii") + cls.executable = work / "test" + command = [ + compiler, "-std=c++17", "-O1", "-g", "-Wall", "-Wextra", "-Werror", + "-Wno-unused-parameter", "-Wno-sign-compare", + "-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-pie", "-no-pie", "-I", str(work), + "-I", str(ROOT / "test/fixtures/region_map_transactions/mocks"), + "-I", str(ROOT / "test/mocks"), "-I", str(ROOT / "src"), + str(FIXTURE), "-o", str(cls.executable), + ] + result = subprocess.run(command, capture_output=True, text=True, timeout=60) + if result.returncode: + cls.temporary.cleanup() + raise AssertionError(result.stdout + result.stderr) + + @classmethod + def tearDownClass(cls): + cls.temporary.cleanup() + + def run_case(self, case): + environment = os.environ.copy() + environment["ASAN_OPTIONS"] = "detect_leaks=1:abort_on_error=1" + environment["UBSAN_OPTIONS"] = "halt_on_error=1:print_stacktrace=1" + result = subprocess.run([str(self.executable), case], capture_output=True, + text=True, timeout=60, env=environment) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("PASS: " + case, result.stdout) + + def test_healthy_production_capacities(self): + self.run_case("healthy") + + def test_repeater_one_byte_destination(self): + self.run_case("repeater-one-byte") + + def test_direct_one_byte_destination(self): + self.run_case("direct-one-byte") + + def test_room_one_byte_destination(self): + self.run_case("room-one-byte") + + def test_null_and_nonpositive_capacities(self): + self.run_case("invalid") + + def test_capacity_and_filter_matrix(self): + self.run_case("matrix") + + def test_role_wrapper_capacity_matrix(self): + self.run_case("wrappers") + + +if __name__ == "__main__": + unittest.main()