factor out accesslist parser to fix a potential out of bound read and add tests to the accesslist unit

4 files changed, 144 insertions, 0 deletions

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 <erdgeist@erdgeist.org>
+   It is considered beerware. Prost. Skol. Cheers or whatever.
+
+   $id$ */
+
+/* System */
+#include <stdlib.h>
+#include <string.h>
+
+/* 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