diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index e3af3aafbf..dd03e8f954 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -21,6 +21,11 @@ jobs: - name: Run Unit Tests run: pio test -e native -vv + env: + # Fail CI on leaks or memory errors reported by ASAN. + ASAN_OPTIONS: detect_leaks=1:halt_on_error=1 + # Stop immediately and print a stack trace for undefined behavior. + UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1 - name: Upload Test Results # Upload test results even if the test step failed. diff --git a/platformio.ini b/platformio.ini index 5c6072b405..e70f21c584 100644 --- a/platformio.ini +++ b/platformio.ini @@ -158,11 +158,18 @@ lib_deps = [env:native] platform = native build_flags = -std=c++17 + ; Preserve stack frames and debug info so ASAN/UBSAN reports are actionable. + -g + -fno-omit-frame-pointer + ; Run native unit tests with AddressSanitizer and UndefinedBehaviorSanitizer. + -fsanitize=address + -fsanitize=undefined -I src -I test/mocks test_build_src = yes build_src_filter = -<*> +<../src/Utils.cpp> + +<../src/helpers/AdvertDataHelpers.cpp> lib_deps = google/googletest @ 1.17.0 diff --git a/src/helpers/AdvertDataHelpers.cpp b/src/helpers/AdvertDataHelpers.cpp index 0e05620ec2..9d5e344ff0 100644 --- a/src/helpers/AdvertDataHelpers.cpp +++ b/src/helpers/AdvertDataHelpers.cpp @@ -29,19 +29,36 @@ AdvertDataParser::AdvertDataParser(const uint8_t app_data[], uint8_t app_data_len) { _name[0] = 0; _lat = _lon = 0; - _flags = app_data[0]; + _flags = 0; _valid = false; _extra1 = _extra2 = 0; - + + if (app_data_len < 1) { + return; + } + + _flags = app_data[0]; int i = 1; if (_flags & ADV_LATLON_MASK) { + if (app_data_len < i + 8) { + return; + } memcpy(&_lat, &app_data[i], 4); i += 4; memcpy(&_lon, &app_data[i], 4); i += 4; + if (_lat < -90000000 || _lat > 90000000 || _lon < -180000000 || _lon > 180000000) { + return; + } } if (_flags & ADV_FEAT1_MASK) { + if (app_data_len < i + 2) { + return; + } memcpy(&_extra1, &app_data[i], 2); i += 2; } if (_flags & ADV_FEAT2_MASK) { + if (app_data_len < i + 2) { + return; + } memcpy(&_extra2, &app_data[i], 2); i += 2; } @@ -84,4 +101,4 @@ void AdvertTimeHelper::formatRelativeTimeDiff(char dest[], int32_t seconds_from_ } } } -} \ No newline at end of file +} diff --git a/test/mocks/Arduino.h b/test/mocks/Arduino.h new file mode 100644 index 0000000000..b7acefbc2a --- /dev/null +++ b/test/mocks/Arduino.h @@ -0,0 +1,18 @@ +#pragma once + +#include +#include +#include +#include +#include + +// Mock Arduino compatibility header for native testing. +// Provides the small libc-backed helpers needed by Arduino-oriented code. +inline char* ltoa(long value, char* buffer, int base) { + if (base == 10) { + snprintf(buffer, 32, "%ld", value); + } else { + buffer[0] = 0; + } + return buffer; +} diff --git a/test/test_helpers/test_advert_data.cpp b/test/test_helpers/test_advert_data.cpp new file mode 100644 index 0000000000..cc1c5a2db5 --- /dev/null +++ b/test/test_helpers/test_advert_data.cpp @@ -0,0 +1,302 @@ +#include +#include +#include + +#include + +#include "helpers/AdvertDataHelpers.h" + +namespace { + +void WriteU8(uint8_t* dest, size_t* offset, uint8_t value) { + dest[(*offset)++] = value; +} + +void WriteI32Le(uint8_t* dest, size_t* offset, int32_t value) { + const uint32_t raw = static_cast(value); + dest[(*offset)++] = static_cast(raw & 0xFF); + dest[(*offset)++] = static_cast((raw >> 8) & 0xFF); + dest[(*offset)++] = static_cast((raw >> 16) & 0xFF); + dest[(*offset)++] = static_cast((raw >> 24) & 0xFF); +} + +void WriteBytes(uint8_t* dest, size_t* offset, const uint8_t* bytes, size_t length) { + memcpy(dest + *offset, bytes, length); + *offset += length; +} + +template +void WriteStringLiteral(uint8_t* dest, size_t* offset, const char (&value)[N]) { + static_assert(N > 0, "string literal must include a null terminator"); + WriteBytes(dest, offset, reinterpret_cast(value), N - 1); +} + +AdvertDataParser Parse(const uint8_t* app_data, size_t app_data_len) { + return AdvertDataParser(app_data, static_cast(app_data_len)); +} + +TEST(AdvertDataParser, ParsesNameAndCoordinates) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: repeater advert with lat/lon followed by a name. + WriteU8(app_data, &offset, ADV_TYPE_REPEATER | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: signed little-endian microdegrees for 37.7749. + WriteI32Le(app_data, &offset, 37774900); + // longitude field: signed little-endian microdegrees for -122.4194. + WriteI32Le(app_data, &offset, -122419400); + // name field: trailing contact name bytes after the coordinate fields. + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + ASSERT_TRUE(parser.isValid()); + EXPECT_EQ(ADV_TYPE_REPEATER, parser.getType()); + ASSERT_TRUE(parser.hasLatLon()); + EXPECT_EQ(37774900, parser.getIntLat()); + EXPECT_EQ(-122419400, parser.getIntLon()); + ASSERT_TRUE(parser.hasName()); + EXPECT_STREQ("dummy-node-name", parser.getName()); +} + +TEST(AdvertDataParser, ParsesCoordinateExtremes) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: sensor advert with both location fields and a name. + WriteU8(app_data, &offset, ADV_TYPE_SENSOR | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: minimum supported latitude, -90.000000 degrees. + WriteI32Le(app_data, &offset, -90000000); + // longitude field: maximum supported longitude, 180.000000 degrees. + WriteI32Le(app_data, &offset, 180000000); + // name field: raw bytes for "dummy-node-name". + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + ASSERT_TRUE(parser.isValid()); + EXPECT_EQ(ADV_TYPE_SENSOR, parser.getType()); + EXPECT_EQ(-90000000, parser.getIntLat()); + EXPECT_EQ(180000000, parser.getIntLon()); + EXPECT_STREQ("dummy-node-name", parser.getName()); +} + +TEST(AdvertDataParser, ParsesPositiveLatitudeAndNegativeLongitudeBoundaries) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: sensor advert with both location fields and a name. + WriteU8(app_data, &offset, ADV_TYPE_SENSOR | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: maximum supported latitude, +90.000000 degrees. + WriteI32Le(app_data, &offset, 90000000); + // longitude field: minimum supported longitude, -180.000000 degrees. + WriteI32Le(app_data, &offset, -180000000); + // name field: raw bytes for "dummy-node-name". + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + ASSERT_TRUE(parser.isValid()); + EXPECT_EQ(90000000, parser.getIntLat()); + EXPECT_EQ(-180000000, parser.getIntLon()); +} + +TEST(AdvertDataParser, ParsesNullIslandCoordinates) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: chat advert with zero-valued coordinates and a name. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: Null Island latitude at exactly 0.000000 degrees. + WriteI32Le(app_data, &offset, 0); + // longitude field: Null Island longitude at exactly 0.000000 degrees. + WriteI32Le(app_data, &offset, 0); + // name field: raw bytes for "dummy-node-name". + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + ASSERT_TRUE(parser.isValid()); + EXPECT_EQ(0, parser.getIntLat()); + EXPECT_EQ(0, parser.getIntLon()); +} + +TEST(AdvertDataParser, ParsesNameWithoutCoordinates) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: chat advert with a name but no GPS fields. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_NAME_MASK); + // name field: contact name with no coordinate payload before it. + WriteStringLiteral(app_data, &offset, "updated-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + ASSERT_TRUE(parser.isValid()); + EXPECT_EQ(ADV_TYPE_CHAT, parser.getType()); + EXPECT_FALSE(parser.hasLatLon()); + ASSERT_TRUE(parser.hasName()); + EXPECT_STREQ("updated-name", parser.getName()); +} + +TEST(AdvertDataParser, RejectsLongitudeOutsideValidRange) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: chat advert with location and name fields present. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: valid latitude so the failure comes from longitude. + WriteI32Le(app_data, &offset, 37774900); + // longitude field: one microdegree above +180.0, which is invalid. + WriteI32Le(app_data, &offset, 180000001); + // name field: parser should reject before the trailing name matters. + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsLongitudeBelowValidRange) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: chat advert with location and name fields present. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: valid latitude so the failure comes from longitude. + WriteI32Le(app_data, &offset, 37774900); + // longitude field: one microdegree below -180.0, which is invalid. + WriteI32Le(app_data, &offset, -180000001); + // name field: included to keep the payload shape consistent. + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsLatitudeOutsideValidRange) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: chat advert with location and name fields present. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: one microdegree above +90.0, which is invalid. + WriteI32Le(app_data, &offset, 90000001); + // longitude field: valid longitude so the failure comes from latitude. + WriteI32Le(app_data, &offset, -122419400); + // name field: included to keep the payload shape consistent. + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsLatitudeBelowValidRange) { + uint8_t app_data[MAX_ADVERT_DATA_SIZE] = {}; + size_t offset = 0; + + // flags/type byte: chat advert with location and name fields present. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: one microdegree below -90.0, which is invalid. + WriteI32Le(app_data, &offset, -90000001); + // longitude field: valid longitude so the failure comes from latitude. + WriteI32Le(app_data, &offset, -122419400); + // name field: included to keep the payload shape consistent. + WriteStringLiteral(app_data, &offset, "dummy-node-name"); + + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsPayloadWithMissingFlagsByte) { + // Backing storage is unused because the advertised app_data length is zero. + const uint8_t app_data[1] = {}; + + const AdvertDataParser parser = Parse(app_data, 0); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsPayloadWithOnlyFlagsByte) { + uint8_t app_data[1] = {}; + size_t offset = 0; + + // flags/type byte: chat advert that claims to carry coordinates and a name. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + + // Pass only the flags byte so no latitude bytes remain. + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsPayloadWithLatitudeButMissingLongitude) { + uint8_t app_data[5] = {}; + size_t offset = 0; + + // flags/type byte: chat advert that claims to carry coordinates and a name. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: complete latitude bytes are present before truncation. + WriteI32Le(app_data, &offset, 37774900); + + // Pass only the flags byte and latitude field so longitude is missing. + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsPayloadOneByteShortOfFullCoordinates) { + uint8_t app_data[8] = {}; + size_t offset = 0; + uint8_t lon_bytes[sizeof(int32_t)] = {}; + size_t lon_offset = 0; + + // flags/type byte: chat advert that claims to carry coordinates and a name. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_LATLON_MASK | ADV_NAME_MASK); + // latitude field: complete latitude bytes are present before truncation. + WriteI32Le(app_data, &offset, 37774900); + // longitude field: only the first three longitude bytes are present before truncation. + WriteI32Le(lon_bytes, &lon_offset, -122419400); + WriteBytes(app_data, &offset, lon_bytes, 3); + + // Pass only the flags byte, latitude field, and three longitude bytes. + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsPayloadWithIncompleteFeat1) { + uint8_t app_data[2] = {}; + size_t offset = 0; + + // flags/type byte: chat advert that claims to carry a two-byte feature field. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_FEAT1_MASK); + // feature field: only the first byte is present before truncation. + WriteU8(app_data, &offset, 0x34); + + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +TEST(AdvertDataParser, RejectsPayloadWithIncompleteFeat2) { + uint8_t app_data[4] = {}; + size_t offset = 0; + + // flags/type byte: chat advert that claims to carry both two-byte feature fields. + WriteU8(app_data, &offset, ADV_TYPE_CHAT | ADV_FEAT1_MASK | ADV_FEAT2_MASK); + // feature 1 field: complete two-byte value before the truncated feature 2 field. + WriteU8(app_data, &offset, 0x34); + WriteU8(app_data, &offset, 0x12); + // feature 2 field: only the first byte is present before truncation. + WriteU8(app_data, &offset, 0x78); + + const AdvertDataParser parser = Parse(app_data, offset); + + EXPECT_FALSE(parser.isValid()); +} + +} // namespace diff --git a/test/test_helpers/test_main.cpp b/test/test_helpers/test_main.cpp new file mode 100644 index 0000000000..697a9d70c0 --- /dev/null +++ b/test/test_helpers/test_main.cpp @@ -0,0 +1,6 @@ +#include + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}