Fix 1-byte OOB write in the old-format cache index import

The .ndx loader in cache_init() reads an entry's host and file into one
line[HTS_URLMAXSIZE*2] buffer with two cache_binput() calls, the second
appending after strlen(line). binput NUL-terminates at s[max], so a record
whose host+file saturate both reads writes the terminating NUL at line[2048],
one past the array. Reachable on --update/--continue against a corrupt or
foreign old-format cache. Cap the second read to the space left in line[].

Drive it through -#test=cacheindex via a new cache_oldndx_selftest that
imports a crafted over-long .ndx; ASan aborts on the pre-fix binput, and the
non-sanitized OK check gates it too.

Also two follow-ups from the #509 fuzz review: fuzz-header now strips every
'\r' per line to match the receive loop (binput drops them), and the biased
regress-truncated-entry.bin seed (which passed regardless of the fix) is
replaced by regress-append-overflow.bin, which reproduces this OOB on the
pre-fix harness.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

fuzz/Makefile.am

@@ -41,4 +41,4 @@ EXTRA_DIST = README.md run-fuzzers.sh \
 	corpus/header/full-response.txt corpus/header/redirect.txt \
 	corpus/cachendx/new-format.txt corpus/cachendx/old-format.txt \
 	corpus/cachendx/regress-overadvance.bin \
-	corpus/cachendx/regress-truncated-entry.bin
+	corpus/cachendx/regress-append-overflow.bin

fuzz/corpus/cachendx/regress-append-overflow.bin

@@ -0,0 +1,5 @@
+9
+CACHE-1.129
+Wed, 01 Jan 2020 00:00:00 GMT2101
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+42

fuzz/corpus/cachendx/regress-truncated-entry.bin

@@ -1,5 +0,0 @@
-8
-CACHE-1.1
-1
-x
-www.example.com

fuzz/fuzz-cachendx.c

@@ -49,13 +49,18 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     char BIGSTK line[HTS_URLMAXSIZE * 2];
     char linepos[256];
     int pos;
+    size_t used, avail;
 
     a = strchr(a + 1, '\n');
     if (a == NULL)
       break;
     a++;
     a += cache_binput(a, end, line, HTS_URLMAXSIZE);
-    a += cache_binput(a, end, line + strlen(line), HTS_URLMAXSIZE);
+    /* cap the append: binput's NUL can land at s[max] (mirrors cache_init) */
+    used = strlen(line);
+    avail = sizeof(line) - used - 1;
+    a += cache_binput(a, end, line + used,
+                      avail < HTS_URLMAXSIZE ? (int) avail : HTS_URLMAXSIZE);
     a += cache_binput(a, end, linepos, 200);
     sscanf(linepos, "%d", &pos);
     (void) pos;

fuzz/fuzz-header.c

@@ -50,9 +50,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
   while (p != NULL && *p != '\0') {
     char *nl = strchr(p, '\n');
     size_t n = (nl != NULL) ? (size_t) (nl - p) : strlen(p);
+    size_t i, len = 0;
 
-    memcpy(line, p, n);
-    line[n] = '\0';
+    /* binput drops every '\r' on the wire; mirror it */
+    for (i = 0; i < n; i++)
+      if (p[i] != '\r')
+        line[len++] = p[i];
+    line[len] = '\0';
     if (first) {
       treatfirstline(&r, line);
       first = 0;

src/htscache.c

@@ -1937,6 +1937,7 @@ void cache_init(cache_back * cache, httrackp * opt) {
             char BIGSTK line[HTS_URLMAXSIZE * 2];
             char linepos[256];
             int pos;
+            size_t used, avail;
 
             const char *const end = cache->use + buffl;
 
@@ -1946,7 +1947,12 @@ void cache_init(cache_back * cache, httrackp * opt) {
                 a++;
                 /* read "host/file" */
                 a += cache_binput(a, end, line, HTS_URLMAXSIZE);
-                a += cache_binput(a, end, line + strlen(line), HTS_URLMAXSIZE);
+                /* cap the append: binput's NUL can land at s[max] */
+                used = strlen(line);
+                avail = sizeof(line) - used - 1;
+                a += cache_binput(a, end, line + used,
+                                  avail < HTS_URLMAXSIZE ? (int) avail
+                                                         : HTS_URLMAXSIZE);
                 /* read position */
                 a += cache_binput(a, end, linepos, 200);
                 sscanf(linepos, "%d", &pos);

src/htscache_selftest.c

@@ -1400,3 +1400,55 @@ int cache_corruption_selftest(httrackp *opt, const char *dir) {
 
   return failures;
 }
+
+/* Old-format (.ndx) import: cache_init scans host+file pairs into one line[]
+   accumulator; a record longer than it must not write past it (ASan gates). */
+int cache_oldndx_selftest(httrackp *opt, const char *dir) {
+  int failures = 0;
+  cache_back cache;
+  FILE *fp;
+  intptr_t pos = 0;
+
+  selftest_tag = "cache-oldndx";
+  golden_setup(opt, dir);
+#ifdef _WIN32
+  mkdir(reconcile_st_path(opt, "hts-cache"));
+#else
+  mkdir(reconcile_st_path(opt, "hts-cache"), HTS_PROTECT_FOLDER);
+#endif
+
+  fp = fopen(reconcile_st_path(opt, "hts-cache/new.ndx"), "wb");
+  assertf(fp != NULL);
+  assertf(cache_wstr(fp, "CACHE-1.1") == 0);
+  assertf(cache_wstr(fp, "Wed, 01 Jan 2020 00:00:00 GMT") == 0);
+  /* a normal record, then one whose host saturates both line[] reads; the
+     oversized one stays last (the scan does not resync after it) */
+  assertf(cache_wstr(fp, "www.example.com\n/index.html\n") == 0);
+  assertf(fwrite("42\n", 1, 3, fp) == 3);
+  {
+    const size_t hostlen = HTS_URLMAXSIZE * 2 + 16;
+    char *rec = malloct(hostlen + sizeof("\n/x\n"));
+
+    memset(rec, 'A', hostlen);
+    memcpy(rec + hostlen, "\n/x\n", sizeof("\n/x\n"));
+    assertf(cache_wstr(fp, rec) == 0);
+    assertf(fwrite("7\n", 1, 2, fp) == 2);
+    freet(rec);
+  }
+  fclose(fp);
+  reconcile_put(opt, "hts-cache/new.dat", 0); /* .ndx alone is not loaded */
+
+  selftest_open_for_read(&cache, opt);
+  if (!coucal_read(cache.hashtable, "www.example.com/index.html", &pos) ||
+      pos != 42) {
+    fprintf(stderr, "cache-oldndx: normal record not indexed (pos=%d)\n",
+            (int) pos);
+    failures++;
+  }
+  if (cache.olddat != NULL) {
+    fclose(cache.olddat);
+    cache.olddat = NULL;
+  }
+  selftest_close(&cache);
+  return failures;
+}

src/htscache_selftest.h

@@ -65,6 +65,11 @@ int cache_reconcile_selftest(httrackp *opt, const char *dir);
    tainting a sibling entry. */
 int cache_corruption_selftest(httrackp *opt, const char *dir);
 
+/* Drive the old-format (.ndx) cache import under <dir> with a record longer
+   than the loader's line accumulator. Returns the failed-check count; the
+   overflow itself is caught by the sanitizer builds. */
+int cache_oldndx_selftest(httrackp *opt, const char *dir);
+
 #endif
 
 #endif

src/htsselftest.c

@@ -1396,9 +1396,10 @@ static int st_cache(httrackp *opt, int argc, char **argv) {
 static int st_cacheindex(httrackp *opt, int argc, char **argv) {
   int fail = 0;
 
-  (void) opt;
-  (void) argc;
-  (void) argv;
+  if (argc < 1) {
+    fprintf(stderr, "cacheindex: needs a directory\n");
+    return 1;
+  }
 
   /* A length prefix that overstates the bytes present must bound the advance
      to the buffer, not trust the declared length. */
@@ -1459,6 +1460,10 @@ static int st_cacheindex(httrackp *opt, int argc, char **argv) {
     freet(buf);
   }
 
+  /* the real loader loop, over a crafted old-format index */
+  if (cache_oldndx_selftest(opt, argv[0]) != 0)
+    fail = 1;
+
   printf("cacheindex: %s\n", fail ? "FAIL" : "OK");
   return fail;
 }
@@ -2286,7 +2291,7 @@ static const struct selftest_entry {
     {"sniff", "<content-type> <hex:..|text>", "MIME magic consistency",
      st_sniff},
     {"cache", "<dir>", "cache read/write round-trip self-test", st_cache},
-    {"cacheindex", "", "cache-index (.ndx) parse must stay in bounds",
+    {"cacheindex", "<dir>", "cache-index (.ndx) parse must stay in bounds",
      st_cacheindex},
     {"cache-golden", "<dir> [regen]", "frozen cache-format read self-test",
      st_cache_golden},

tests/01_engine-cacheindex.test

@@ -4,7 +4,10 @@
 set -euo pipefail
 
 # Cache-index (.ndx) parse must stay inside the buffer on a corrupt length
-# prefix (ASan aborts here on the pre-fix binary; the OK check gates the
-# non-sanitized build too).
+# prefix or an oversized record (ASan aborts here on the pre-fix binary; the
+# OK check gates the non-sanitized build too).
 
-test "$(httrack -O /dev/null -#test=cacheindex)" == 'cacheindex: OK'
+dir=$(mktemp -d)
+trap 'rm -rf "$dir"' EXIT
+
+test "$(httrack "-#test=cacheindex" "$dir")" == 'cacheindex: OK'