diff --git a/src/modules/AtakPluginModule.cpp b/src/modules/AtakPluginModule.cpp index a51ef54c3..416c36cb6 100644 --- a/src/modules/AtakPluginModule.cpp +++ b/src/modules/AtakPluginModule.cpp @@ -10,6 +10,23 @@ AtakPluginModule *atakPluginModule; +/** + * Get actual string length for nanopb char array fields. + * Nanopb stores strings as fixed-size char arrays that may contain embedded nulls. + * strlen() would stop at the first null, but we need to find the last non-null character. + * This is critical for Android UIDs that can contain 0x00 bytes (e.g., ANDROID-e7e455b40002429d). + */ +static size_t pb_string_length(const char *str, size_t max_len) +{ + size_t len = 0; + for (size_t i = 0; i < max_len; i++) { + if (str[i] != '\0') { + len = i + 1; + } + } + return len; +} + AtakPluginModule::AtakPluginModule() : ProtobufModule("atak", meshtastic_PortNum_ATAK_PLUGIN, &meshtastic_TAKPacket_msg), concurrency::OSThread("AtakPlugin") { @@ -70,16 +87,17 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast auto compressed = cloneTAKPacketData(t); compressed.is_compressed = true; if (t->has_contact) { - auto length = unishox2_compress_lines(t->contact.callsign, strlen(t->contact.callsign), compressed.contact.callsign, - sizeof(compressed.contact.callsign) - 1, USX_PSET_DFLT, NULL); + auto length = unishox2_compress_lines( + t->contact.callsign, pb_string_length(t->contact.callsign, sizeof(t->contact.callsign)), + compressed.contact.callsign, sizeof(compressed.contact.callsign) - 1, USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Compress overflow contact.callsign. Revert to uncompressed packet"); return; } LOG_DEBUG("Compressed callsign: %d bytes", length); - length = unishox2_compress_lines(t->contact.device_callsign, strlen(t->contact.device_callsign), - compressed.contact.device_callsign, sizeof(compressed.contact.device_callsign) - 1, - USX_PSET_DFLT, NULL); + length = unishox2_compress_lines( + t->contact.device_callsign, pb_string_length(t->contact.device_callsign, sizeof(t->contact.device_callsign)), + compressed.contact.device_callsign, sizeof(compressed.contact.device_callsign) - 1, USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Compress overflow contact.device_callsign. Revert to uncompressed packet"); return; @@ -87,9 +105,11 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast LOG_DEBUG("Compressed device_callsign: %d bytes", length); } if (t->which_payload_variant == meshtastic_TAKPacket_chat_tag) { - auto length = unishox2_compress_lines(t->payload_variant.chat.message, strlen(t->payload_variant.chat.message), - compressed.payload_variant.chat.message, - sizeof(compressed.payload_variant.chat.message) - 1, USX_PSET_DFLT, NULL); + auto length = unishox2_compress_lines( + t->payload_variant.chat.message, + pb_string_length(t->payload_variant.chat.message, sizeof(t->payload_variant.chat.message)), + compressed.payload_variant.chat.message, sizeof(compressed.payload_variant.chat.message) - 1, USX_PSET_DFLT, + NULL); if (length < 0) { LOG_WARN("Compress overflow chat.message. Revert to uncompressed packet"); return; @@ -98,9 +118,9 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast if (t->payload_variant.chat.has_to) { compressed.payload_variant.chat.has_to = true; - length = unishox2_compress_lines(t->payload_variant.chat.to, strlen(t->payload_variant.chat.to), - compressed.payload_variant.chat.to, - sizeof(compressed.payload_variant.chat.to) - 1, USX_PSET_DFLT, NULL); + length = unishox2_compress_lines( + t->payload_variant.chat.to, pb_string_length(t->payload_variant.chat.to, sizeof(t->payload_variant.chat.to)), + compressed.payload_variant.chat.to, sizeof(compressed.payload_variant.chat.to) - 1, USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Compress overflow chat.to. Revert to uncompressed packet"); return; @@ -110,9 +130,11 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast if (t->payload_variant.chat.has_to_callsign) { compressed.payload_variant.chat.has_to_callsign = true; - length = unishox2_compress_lines(t->payload_variant.chat.to_callsign, strlen(t->payload_variant.chat.to_callsign), - compressed.payload_variant.chat.to_callsign, - sizeof(compressed.payload_variant.chat.to_callsign) - 1, USX_PSET_DFLT, NULL); + length = unishox2_compress_lines( + t->payload_variant.chat.to_callsign, + pb_string_length(t->payload_variant.chat.to_callsign, sizeof(t->payload_variant.chat.to_callsign)), + compressed.payload_variant.chat.to_callsign, sizeof(compressed.payload_variant.chat.to_callsign) - 1, + USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Compress overflow chat.to_callsign. Revert to uncompressed packet"); return; @@ -134,18 +156,18 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast auto uncompressed = cloneTAKPacketData(t); uncompressed.is_compressed = false; if (t->has_contact) { - auto length = - unishox2_decompress_lines(t->contact.callsign, strlen(t->contact.callsign), uncompressed.contact.callsign, - sizeof(uncompressed.contact.callsign) - 1, USX_PSET_DFLT, NULL); + auto length = unishox2_decompress_lines( + t->contact.callsign, pb_string_length(t->contact.callsign, sizeof(t->contact.callsign)), + uncompressed.contact.callsign, sizeof(uncompressed.contact.callsign) - 1, USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Decompress overflow contact.callsign. Bailing out"); return; } LOG_DEBUG("Decompressed callsign: %d bytes", length); - length = unishox2_decompress_lines(t->contact.device_callsign, strlen(t->contact.device_callsign), - uncompressed.contact.device_callsign, - sizeof(uncompressed.contact.device_callsign) - 1, USX_PSET_DFLT, NULL); + length = unishox2_decompress_lines( + t->contact.device_callsign, pb_string_length(t->contact.device_callsign, sizeof(t->contact.device_callsign)), + uncompressed.contact.device_callsign, sizeof(uncompressed.contact.device_callsign) - 1, USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Decompress overflow contact.device_callsign. Bailing out"); return; @@ -153,9 +175,11 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast LOG_DEBUG("Decompressed device_callsign: %d bytes", length); } if (uncompressed.which_payload_variant == meshtastic_TAKPacket_chat_tag) { - auto length = unishox2_decompress_lines(t->payload_variant.chat.message, strlen(t->payload_variant.chat.message), - uncompressed.payload_variant.chat.message, - sizeof(uncompressed.payload_variant.chat.message) - 1, USX_PSET_DFLT, NULL); + auto length = unishox2_decompress_lines( + t->payload_variant.chat.message, + pb_string_length(t->payload_variant.chat.message, sizeof(t->payload_variant.chat.message)), + uncompressed.payload_variant.chat.message, sizeof(uncompressed.payload_variant.chat.message) - 1, USX_PSET_DFLT, + NULL); if (length < 0) { LOG_WARN("Decompress overflow chat.message. Bailing out"); return; @@ -164,9 +188,9 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast if (t->payload_variant.chat.has_to) { uncompressed.payload_variant.chat.has_to = true; - length = unishox2_decompress_lines(t->payload_variant.chat.to, strlen(t->payload_variant.chat.to), - uncompressed.payload_variant.chat.to, - sizeof(uncompressed.payload_variant.chat.to) - 1, USX_PSET_DFLT, NULL); + length = unishox2_decompress_lines( + t->payload_variant.chat.to, pb_string_length(t->payload_variant.chat.to, sizeof(t->payload_variant.chat.to)), + uncompressed.payload_variant.chat.to, sizeof(uncompressed.payload_variant.chat.to) - 1, USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Decompress overflow chat.to. Bailing out"); return; @@ -176,10 +200,11 @@ void AtakPluginModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtast if (t->payload_variant.chat.has_to_callsign) { uncompressed.payload_variant.chat.has_to_callsign = true; - length = - unishox2_decompress_lines(t->payload_variant.chat.to_callsign, strlen(t->payload_variant.chat.to_callsign), - uncompressed.payload_variant.chat.to_callsign, - sizeof(uncompressed.payload_variant.chat.to_callsign) - 1, USX_PSET_DFLT, NULL); + length = unishox2_decompress_lines( + t->payload_variant.chat.to_callsign, + pb_string_length(t->payload_variant.chat.to_callsign, sizeof(t->payload_variant.chat.to_callsign)), + uncompressed.payload_variant.chat.to_callsign, sizeof(uncompressed.payload_variant.chat.to_callsign) - 1, + USX_PSET_DFLT, NULL); if (length < 0) { LOG_WARN("Decompress overflow chat.to_callsign. Bailing out"); return; diff --git a/test/test_atak/test_main.cpp b/test/test_atak/test_main.cpp new file mode 100644 index 000000000..59fffcbe5 --- /dev/null +++ b/test/test_atak/test_main.cpp @@ -0,0 +1,230 @@ +#include +#include + +/** + * Get actual string length for nanopb char array fields. + * Nanopb stores strings as fixed-size char arrays that may contain embedded nulls. + * strlen() would stop at the first null, but we need to find the last non-null character. + * This is critical for Android UIDs that can contain 0x00 bytes (e.g., ANDROID-e7e455b40002429d). + */ +static size_t pb_string_length(const char *str, size_t max_len) +{ + size_t len = 0; + for (size_t i = 0; i < max_len; i++) { + if (str[i] != '\0') { + len = i + 1; + } + } + return len; +} + +void setUp(void) +{ + // set stuff up here +} + +void tearDown(void) +{ + // clean stuff up here +} + +/** + * Test normal string without embedded nulls + * Should behave the same as strlen() for regular strings + */ +void test_normal_string(void) +{ + char test_str[32] = "Hello World"; + size_t expected = 11; // strlen("Hello World") + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(expected, result); +} + +/** + * Test empty string + * Should return 0 for empty string + */ +void test_empty_string(void) +{ + char test_str[32] = ""; + size_t expected = 0; + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(expected, result); +} + +/** + * Test string with only trailing nulls + * Common case - string followed by null padding + */ +void test_trailing_nulls(void) +{ + char test_str[32] = {0}; + strcpy(test_str, "Test"); + // test_str is now: "Test\0\0\0\0..." (4 chars + 28 nulls) + size_t expected = 4; + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(expected, result); +} + +/** + * Test string with embedded null byte + * This is the critical bug case - strlen() would truncate at first null + */ +void test_embedded_null(void) +{ + char test_str[32] = {0}; + // Create string "ABC\0XYZ" (embedded null after C) + test_str[0] = 'A'; + test_str[1] = 'B'; + test_str[2] = 'C'; + test_str[3] = '\0'; // embedded null + test_str[4] = 'X'; + test_str[5] = 'Y'; + test_str[6] = 'Z'; + // Rest is already null from initialization + + // strlen would return 3, but pb_string_length should return 7 + size_t strlen_result = strlen(test_str); + size_t pb_result = pb_string_length(test_str, sizeof(test_str)); + + TEST_ASSERT_EQUAL_size_t(3, strlen_result); // strlen stops at first null + TEST_ASSERT_EQUAL_size_t(7, pb_result); // pb_string_length finds last non-null +} + +/** + * Test Android UID with embedded null bytes + * Real-world case from bug report: ANDROID-e7e455b40002429d + * The "00" in the UID represents 0x00 bytes that were truncating the string + */ +void test_android_uid_pattern(void) +{ + char test_str[32] = {0}; + // Simulate "ANDROID-e7e455b4" + 0x00 + 0x00 + "2429d" + const char part1[] = "ANDROID-e7e455b4"; + strcpy(test_str, part1); + size_t pos = strlen(part1); + test_str[pos] = '\0'; // embedded null + test_str[pos + 1] = '\0'; // another embedded null + strcpy(test_str + pos + 2, "2429d"); + + // The full UID should be 24 characters + size_t strlen_result = strlen(test_str); + size_t pb_result = pb_string_length(test_str, sizeof(test_str)); + + TEST_ASSERT_EQUAL_size_t(16, strlen_result); // strlen truncates to "ANDROID-e7e455b4" + TEST_ASSERT_EQUAL_size_t(23, pb_result); // pb_string_length gets full length +} + +/** + * Test string with multiple embedded nulls + * Edge case with several null bytes scattered through the string + */ +void test_multiple_embedded_nulls(void) +{ + char test_str[32] = {0}; + // Create "A\0B\0C\0D" (3 embedded nulls) + test_str[0] = 'A'; + test_str[1] = '\0'; + test_str[2] = 'B'; + test_str[3] = '\0'; + test_str[4] = 'C'; + test_str[5] = '\0'; + test_str[6] = 'D'; + + size_t strlen_result = strlen(test_str); + size_t pb_result = pb_string_length(test_str, sizeof(test_str)); + + TEST_ASSERT_EQUAL_size_t(1, strlen_result); // strlen stops at first null + TEST_ASSERT_EQUAL_size_t(7, pb_result); // pb_string_length finds all chars +} + +/** + * Test buffer completely filled with non-null characters + * Edge case where string uses entire buffer + */ +void test_full_buffer(void) +{ + char test_str[8]; + // Fill entire buffer with 'X' + memset(test_str, 'X', sizeof(test_str)); + + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(8, result); +} + +/** + * Test buffer with all nulls + * Should return 0 + */ +void test_all_nulls(void) +{ + char test_str[32] = {0}; + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(0, result); +} + +/** + * Test single character followed by nulls + * Minimal non-empty case + */ +void test_single_char(void) +{ + char test_str[32] = {0}; + test_str[0] = 'X'; + + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(1, result); +} + +/** + * Test callsign field typical size + * Test with typical ATAK callsign field size (64 bytes) + */ +void test_callsign_field_size(void) +{ + char test_str[64] = {0}; + strcpy(test_str, "CALLSIGN-123"); + + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(12, result); +} + +/** + * Test with data at end of buffer + * String with embedded null and data at very end + */ +void test_data_at_buffer_end(void) +{ + char test_str[10] = {0}; + test_str[0] = 'A'; + test_str[1] = '\0'; + test_str[8] = 'Z'; // Data near end + test_str[9] = 'X'; // Data at end + + size_t result = pb_string_length(test_str, sizeof(test_str)); + TEST_ASSERT_EQUAL_size_t(10, result); // Should find the 'X' at position 9 +} + +void setup() +{ + // NOTE!!! Wait for >2 secs + // if board doesn't support software reset via Serial.DTR/RTS + delay(10); + delay(2000); + + UNITY_BEGIN(); + RUN_TEST(test_normal_string); + RUN_TEST(test_empty_string); + RUN_TEST(test_trailing_nulls); + RUN_TEST(test_embedded_null); + RUN_TEST(test_android_uid_pattern); + RUN_TEST(test_multiple_embedded_nulls); + RUN_TEST(test_full_buffer); + RUN_TEST(test_all_nulls); + RUN_TEST(test_single_char); + RUN_TEST(test_callsign_field_size); + RUN_TEST(test_data_at_buffer_end); + exit(UNITY_END()); +} + +void loop() {}