From ba54aca3a47df0e8c7afde968bd5f7479372a404 Mon Sep 17 00:00:00 2001 From: Dirk Engling Date: Sat, 24 Oct 2026 03:43:34 +0200 Subject: factor out accesslist parser to fix a potential out of bound read and add tests to the accesslist unit --- tests/runner.c | 1 + tests/stubs.c | 20 +++++++++ tests/t_accesslist.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/test.h | 1 + 4 files changed, 144 insertions(+) create mode 100644 tests/t_accesslist.c (limited to 'tests') diff --git a/tests/runner.c b/tests/runner.c index c9385f0..bf4fbde 100644 --- a/tests/runner.c +++ b/tests/runner.c @@ -19,6 +19,7 @@ static const struct module modules[] = {{"scan_urlencoded_query", test_scan_urle {"ot_vector", test_vector}, {"ot_random", test_random}, {"ot_hex", test_hex}, + {"ot_accesslist", test_accesslist}, {NULL, NULL}}; /* Run every module, or only those whose name contains argv[1] */ diff --git a/tests/stubs.c b/tests/stubs.c index ae40306..febdd7b 100644 --- a/tests/stubs.c +++ b/tests/stubs.c @@ -12,3 +12,23 @@ /* ot_vector.c calls this from vector_clean_list, which no test drives yet. */ void free_peerlist(ot_peerlist *peer_list) { (void)peer_list; } + +/* ot_accesslist.c is built with WANT_ACCESSLIST_BLACK so that the hash parser + is reachable. These four belong to accesslist_readfile and the reload + worker, neither of which any test drives. */ +time_t g_now_seconds; +volatile int g_opentracker_running; + +const char *mmap_read(const char *filename, size_t *filesize); +const char *mmap_read(const char *filename, size_t *filesize) { + (void)filename; + *filesize = 0; + return NULL; +} + +int mmap_unmap(const char *mapped, size_t maplen); +int mmap_unmap(const char *mapped, size_t maplen) { + (void)mapped; + (void)maplen; + return 0; +} diff --git a/tests/t_accesslist.c b/tests/t_accesslist.c new file mode 100644 index 0000000..9925acd --- /dev/null +++ b/tests/t_accesslist.c @@ -0,0 +1,122 @@ +/* This software was written by Dirk Engling + It is considered beerware. Prost. Skol. Cheers or whatever. + + $id$ */ + +/* System */ +#include +#include + +/* Opentracker */ +#include "ot_accesslist.h" +#include "test.h" + +#define A "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +#define B "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB" +#define C "CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC" + +static ot_hash dest[8]; + +/* The parser is handed a mapped file, so it must never look at a byte beyond + the length it was given. These cases place a hex digit immediately after + that length: a parser that reads one byte too far sees it, decides the hash + runs on, and silently drops the last entry. */ + +static void case_empty(void) { + test_case("an empty buffer yields nothing"); + CHECK_INT(accesslist_parse_hashes(dest, 8, "", 0), 0); +} + +static void case_single_with_newline(void) { + const char buf[] = A "\n"; + + test_case("one hash followed by a newline"); + CHECK_INT(accesslist_parse_hashes(dest, 8, buf, 41), 1); +} + +static void case_single_without_newline(void) { + const char buf[] = A "A"; + + test_case("one hash ending the buffer, with a hex digit just past the end"); + CHECK_INT(accesslist_parse_hashes(dest, 8, buf, 40), 1); +} + +static void case_several_last_without_newline(void) { + const char buf[] = A "\n" B "\n" C "C"; + + test_case("a final hash without a trailing newline still counts"); + CHECK_INT(accesslist_parse_hashes(dest, 8, buf, 3 * 40 + 2), 3); +} + +static void case_run_on_hash_is_rejected(void) { + const char buf[] = A "A\n"; + + test_case("forty one hex digits are not a hash"); + CHECK_INT(accesslist_parse_hashes(dest, 8, buf, 42), 0); +} + +static void case_junk_lines_skipped(void) { + const char buf[] = "# a comment\n" A "\nnot a hash\n" B "\n"; + + test_case("lines that are not hashes are ignored"); + CHECK_INT(accesslist_parse_hashes(dest, 8, buf, sizeof(buf) - 1), 2); +} + +static void case_trailing_junk_on_line(void) { + const char buf[] = A " some comment\n" B "\n"; + + test_case("a hash followed by other text on the same line still counts"); + CHECK_INT(accesslist_parse_hashes(dest, 8, buf, sizeof(buf) - 1), 2); +} + +static void case_capacity_is_respected(void) { + const char buf[] = A "\n" B "\n" C "\n"; + + test_case("no more than capacity hashes are written"); + CHECK_INT(accesslist_parse_hashes(dest, 2, buf, sizeof(buf) - 1), 2); +} + +static void case_zero_capacity_never_writes(void) { + const char buf[] = A "\n"; + + test_case("a zero capacity tolerates a null destination"); + CHECK_INT(accesslist_parse_hashes(NULL, 0, buf, 41), 0); +} + +static void case_decoded_value(void) { + const char buf[] = "000102030405060708090A0B0C0D0E0F10111213\n"; + const uint8_t want[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}; + + test_case("the hash is decoded, not merely counted"); + CHECK_INT(accesslist_parse_hashes(dest, 8, buf, 41), 1); + CHECK_MEM(dest[0], want, sizeof(want)); +} + +static void case_capacity_bound_holds(void) { + /* (len + 1) / 41 is what accesslist_readfile allocates. Pack a buffer as + densely as the parser allows and check the bound is not exceeded. */ + char buf[41 * 4]; + size_t len, capacity, found; + + test_case("the capacity formula bounds what the parser can produce"); + memcpy(buf, A "\n" B "\n" C "\n" A, 4 * 40 + 3); + len = 4 * 40 + 3; + capacity = (len + 1) / 41; + found = accesslist_parse_hashes(dest, 8, buf, len); + CHECK_INT(found, 4); + CHECK(found <= capacity); +} + +void test_accesslist(void) { + case_empty(); + case_single_with_newline(); + case_single_without_newline(); + case_several_last_without_newline(); + case_run_on_hash_is_rejected(); + case_junk_lines_skipped(); + case_trailing_junk_on_line(); + case_capacity_is_respected(); + case_zero_capacity_never_writes(); + case_decoded_value(); + case_capacity_bound_holds(); +} diff --git a/tests/test.h b/tests/test.h index 6af0083..8be624b 100644 --- a/tests/test.h +++ b/tests/test.h @@ -81,5 +81,6 @@ void test_iovec(void); void test_vector(void); void test_random(void); void test_hex(void); +void test_accesslist(void); #endif -- cgit v1.3