Harden the zip cache read path against a corrupt X-Size

A tampered X-Size in a zip cache entry wrapped into an oversized malloc
(an allocation-size-too-big abort under ASan). The alloc casts to int
(malloct((int) r.size + 1)), so besides a negative size, any positive
value at or above INT_MAX truncates negative and wraps too; reject the
whole out-of-range span before the load.

-#test=cache-corrupt byte-injects a real two-entry zip (bad/oversized/
negative X-Size, blanked X-In-Cache, smashed local header, garbled
deflate) and checks each entry degrades to STATUSCODE_INVALID in the same
read session as its intact sibling, so one corrupt entry never taints the
cache. Only the live zip format is covered; the legacy .dat reader is dead
and slated for removal.

Re-lands the change orphaned when it was merged into its stacked base
branch instead of master.

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

src/htscache.c

@@ -40,6 +40,7 @@ Please visit our Website: http://www.httrack.com
 #include "htscore.h"
 #include "htsbasenet.h"
 #include "htsmd5.h"
+#include <limits.h>
 #include <time.h>
 
 #include "htszlib.h"
@@ -768,6 +769,15 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
             strlcpybuff(return_save, previous_save, HTS_URLMAXSIZE * 2);
           }
 
+          /* A tampered X-Size must be rejected before the size-driven malloc.
+             The alloc casts to int (malloct((int) r.size + 1)), so bound it to
+             [0, INT_MAX): a negative value, or a positive one whose (int) cast
+             truncates negative, would otherwise wrap to a huge allocation. */
+          if (r.size < 0 || r.size >= INT_MAX) {
+            r.statuscode = STATUSCODE_INVALID;
+            strcpybuff(r.msg, "Cache Read Error : Bad Size");
+          }
+
           /* Complete fields */
           r.totalsize = r.size;
           r.adr = NULL;
@@ -794,7 +804,8 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
             }                   // otherwise, the ZIP file is supposed to be consistent with data.
           }
           /* Read data ? */
-          else {                /* ne pas lire uniquement header */
+          else if (r.statuscode !=
+                   STATUSCODE_INVALID) { /* ne pas lire uniquement header */
             int ok = 0;
 
 #if HTS_DIRECTDISK

src/htscache_selftest.c

@@ -919,3 +919,195 @@ int cache_reconcile_selftest(httrackp *opt, const char *dir) {
   reconcile_wipe(opt);
   return failures;
 }
+
+/* --- read-side corruption injection --------------------------------------- */
+
+/* canary read back intact after each corruption; victim gets the byte surgery
+ */
+#define CORRUPT_ADR "corrupt.example.com"
+static char corrupt_body_a[33 + 1];
+static char corrupt_body_b[44 + 1];
+
+/* Write a fresh two-entry cache: /canary.html then /victim.html. */
+static void corrupt_build(httrackp *opt) {
+  cache_back cache;
+
+  memset(corrupt_body_a, 'a', sizeof(corrupt_body_a) - 1);
+  memset(corrupt_body_b, 'b', sizeof(corrupt_body_b) - 1);
+  remove(reconcile_st_path(opt, "hts-cache/new.zip"));
+  remove(reconcile_st_path(opt, "hts-cache/old.zip"));
+  selftest_open_for_write(&cache, opt);
+  store_entry(opt, &cache, CORRUPT_ADR, "/canary.html", "canary.html", 200,
+              "OK", "text/html", "utf-8", "", "", "", "", corrupt_body_a,
+              strlen(corrupt_body_a));
+  store_entry(opt, &cache, CORRUPT_ADR, "/victim.html", "victim.html", 200,
+              "OK", "text/html", "utf-8", "", "", "", "", corrupt_body_b,
+              strlen(corrupt_body_b));
+  selftest_close(&cache);
+}
+
+/* Like corrupt_build, but the victim carries a 20-char Etag whose header line
+   is later overwritten with a forged oversized X-Size (same byte length). */
+static void corrupt_build_etag(httrackp *opt) {
+  cache_back cache;
+
+  memset(corrupt_body_a, 'a', sizeof(corrupt_body_a) - 1);
+  memset(corrupt_body_b, 'b', sizeof(corrupt_body_b) - 1);
+  remove(reconcile_st_path(opt, "hts-cache/new.zip"));
+  remove(reconcile_st_path(opt, "hts-cache/old.zip"));
+  selftest_open_for_write(&cache, opt);
+  store_entry(opt, &cache, CORRUPT_ADR, "/canary.html", "canary.html", 200,
+              "OK", "text/html", "utf-8", "", "", "", "", corrupt_body_a,
+              strlen(corrupt_body_a));
+  store_entry(opt, &cache, CORRUPT_ADR, "/victim.html", "victim.html", 200,
+              "OK", "text/html", "utf-8", "", "AAAAAAAAAAAAAAAAAAAA", "", "",
+              corrupt_body_b, strlen(corrupt_body_b));
+  selftest_close(&cache);
+}
+
+/* Patch the nth of total occurrences of pat (same-length rep) in new.zip. */
+static void corrupt_patch(httrackp *opt, const char *pat, size_t patlen,
+                          const char *rep, size_t nth, size_t total) {
+  LLint fsz = 0;
+  char *data = readfile2(reconcile_st_path(opt, "hts-cache/new.zip"), &fsz);
+  const size_t n = (size_t) fsz;
+  size_t k, hits = 0, at = 0;
+  FILE *fp;
+
+  assertf(data != NULL);
+  for (k = 0; k + patlen <= n; k++) {
+    if (memcmp(data + k, pat, patlen) == 0) {
+      hits++;
+      if (hits == nth)
+        at = k;
+    }
+  }
+  assertf(hits == total);
+  memcpy(data + at, rep, patlen);
+  fp = fopen(reconcile_st_path(opt, "hts-cache/new.zip"), "wb");
+  assertf(fp != NULL);
+  assertf(fwrite(data, 1, n, fp) == n);
+  fclose(fp);
+  freet(data);
+}
+
+/* Garbage the first bytes of the victim's deflated data (2nd local header). */
+static void corrupt_victim_body(httrackp *opt) {
+  LLint fsz = 0;
+  char *data = readfile2(reconcile_st_path(opt, "hts-cache/new.zip"), &fsz);
+  const size_t n = (size_t) fsz;
+  size_t k, hits = 0, off = 0;
+  FILE *fp;
+
+  assertf(data != NULL);
+  for (k = 0; k + 4 <= n; k++) {
+    if (memcmp(data + k, "PK\x03\x04", 4) == 0 && ++hits == 2) {
+      const size_t namelen =
+          (unsigned char) data[k + 26] | ((unsigned char) data[k + 27] << 8);
+      const size_t extralen =
+          (unsigned char) data[k + 28] | ((unsigned char) data[k + 29] << 8);
+
+      off = k + 30 + namelen + extralen;
+    }
+  }
+  assertf(hits == 2);
+  assertf(off != 0 && off + 4 <= n);
+  memset(data + off, 0xFF, 4);
+  fp = fopen(reconcile_st_path(opt, "hts-cache/new.zip"), "wb");
+  assertf(fp != NULL);
+  assertf(fwrite(data, 1, n, fp) == n);
+  fclose(fp);
+  freet(data);
+}
+
+/* Read the corrupt /victim.html and, in the SAME read session, the intact
+   /canary.html: the victim must be rejected (wantmsg pins which path) and the
+   canary must still decode byte-exact, proving one bad entry never taints a
+   sibling read. */
+static int corrupt_expect_victim(httrackp *opt, const char *wantmsg,
+                                 const char *what) {
+  cache_back cache;
+  htsblk v, c;
+  char BIGSTK lv[HTS_URLMAXSIZE * 2];
+  char BIGSTK lc[HTS_URLMAXSIZE * 2];
+  int fail = 0;
+
+  selftest_open_for_read(&cache, opt);
+  lv[0] = lc[0] = '\0';
+  v = cache_readex(opt, &cache, CORRUPT_ADR, "/victim.html", "", lv, NULL, 1);
+  if (v.statuscode != STATUSCODE_INVALID) {
+    fprintf(stderr, "%s: %s: victim: statuscode is %d, expected %d\n",
+            selftest_tag, what, v.statuscode, STATUSCODE_INVALID);
+    fail++;
+  }
+  if (wantmsg != NULL && strcmp(v.msg, wantmsg) != 0) {
+    fprintf(stderr, "%s: %s: victim: msg is '%s', expected '%s'\n",
+            selftest_tag, what, v.msg, wantmsg);
+    fail++;
+  }
+  c = cache_readex(opt, &cache, CORRUPT_ADR, "/canary.html", "", lc, NULL, 1);
+  if (c.statuscode != 200 || c.adr == NULL ||
+      c.size != (LLint) strlen(corrupt_body_a) ||
+      memcmp(c.adr, corrupt_body_a, strlen(corrupt_body_a)) != 0) {
+    fprintf(stderr, "%s: %s: canary tainted (status %d)\n", selftest_tag, what,
+            c.statuscode);
+    fail++;
+  }
+  if (v.adr != NULL)
+    freet(v.adr);
+  if (c.adr != NULL)
+    freet(c.adr);
+  selftest_close(&cache);
+  return fail;
+}
+
+/* One zip corruption case: build, patch, then check victim+canary in-session.
+ */
+static int corrupt_case_zip(httrackp *opt, const char *pat, const char *rep,
+                            size_t nth, size_t total, const char *wantmsg,
+                            const char *what) {
+  corrupt_build(opt);
+  corrupt_patch(opt, pat, strlen(pat), rep, nth, total);
+  return corrupt_expect_victim(opt, wantmsg, what);
+}
+
+int cache_corruption_selftest(httrackp *opt, const char *dir) {
+  int failures = 0;
+
+  selftest_tag = "cache-corrupt";
+  golden_setup(opt, dir);
+
+  failures +=
+      corrupt_case_zip(opt, "X-Size: 44", "X-Size: 99", 1, 1,
+                       "Cache Read Error : Read Data", "oversized X-Size");
+  failures +=
+      corrupt_case_zip(opt, "X-Size: 44", "X-Size: -4", 1, 1,
+                       "Cache Read Error : Bad Size", "negative X-Size");
+  /* both entries carry the line; the victim's is the second */
+  failures += corrupt_case_zip(opt, "X-In-Cache: 1", "X-In-Cache: 0", 2, 2,
+                               "Previous cache file not found (empty filename)",
+                               "blanked X-In-Cache");
+  /* smashed local file header: the entry is dropped at index load */
+  failures +=
+      corrupt_case_zip(opt, "PK\x03\x04", "XK\x03\x04", 2, 2,
+                       "File Cache Entry Not Found", "smashed local header");
+
+  corrupt_build(opt);
+  corrupt_victim_body(opt);
+  failures += corrupt_expect_victim(opt, "Cache Read Error : Read Data",
+                                    "garbled deflate stream");
+
+  /* An X-Size above INT_MAX is positive as int64 (slips a bare sign check) but
+     truncates negative in the (int) cast the malloc uses: a wraparound alloc.
+     cache_add asserts size fits an int, so such a value only reaches the reader
+     from a corrupt/foreign cache; inject it by overwriting the victim's long
+     Etag line with a same-length forged X-Size line (the parser keeps the last
+     X-Size it sees), keeping the zip byte-length and offsets intact. */
+  corrupt_build_etag(opt);
+  corrupt_patch(opt, "Etag: AAAAAAAAAAAAAAAAAAAA", 26,
+                "X-Size: 2147483648AAAAAAAA", 1, 1);
+  failures += corrupt_expect_victim(opt, "Cache Read Error : Bad Size",
+                                    "X-Size above INT_MAX");
+
+  return failures;
+}

src/htscache_selftest.h

@@ -60,6 +60,11 @@ int cache_write_failure_selftest(httrackp *opt, const char *dir);
    under <dir>. Returns the failed-check count. */
 int cache_reconcile_selftest(httrackp *opt, const char *dir);
 
+/* Inject read-side corruption (zip byte surgery: bad size, header, deflate)
+   under <dir> and assert every case degrades to STATUSCODE_INVALID without
+   tainting a sibling entry. */
+int cache_corruption_selftest(httrackp *opt, const char *dir);
+
 #endif
 
 #endif

src/htsselftest.c

@@ -1347,6 +1347,18 @@ static int st_cache_writefail(httrackp *opt, int argc, char **argv) {
   return err;
 }
 
+static int st_cache_corrupt(httrackp *opt, int argc, char **argv) {
+  int err;
+
+  if (argc < 1) {
+    fprintf(stderr, "cache-corrupt: needs a directory\n");
+    return 1;
+  }
+  err = cache_corruption_selftest(opt, argv[0]);
+  printf("cache-corrupt: %s\n", err ? "FAIL" : "OK");
+  return err;
+}
+
 static int st_reconcile(httrackp *opt, int argc, char **argv) {
   int err;
 
@@ -2133,6 +2145,8 @@ static const struct selftest_entry {
      st_cache_writefail},
     {"reconcile", "<dir>", "cache generation reconcile policy self-test",
      st_reconcile},
+    {"cache-corrupt", "<dir>", "cache read-side corruption self-test",
+     st_cache_corrupt},
     {"dns", "", "DNS resolver/cache self-test", st_dns},
     {"cookies", "", "cookie request-header self-test", st_cookies},
     {"useragent", "", "default User-Agent self-test", st_useragent},

tests/01_zlib-cache-corrupt.test

@@ -0,0 +1,19 @@
+#!/bin/bash
+#
+# Read-side cache corruption (httrack -#test=cache-corrupt <dir>): zip byte
+# surgery (bad/oversized X-Size, blanked X-In-Cache, smashed header, garbled
+# deflate) must each be rejected per-entry, never crash, never taint the sibling.
+
+set -eu
+
+dir=$(mktemp -d)
+trap 'rm -rf "$dir"' EXIT
+
+# the smashed-header case logs expected "Corrupted cache entry" warnings on
+# stdout; the verdict is the last line
+out=$(httrack -#test=cache-corrupt "$dir" 2>/dev/null | tail -n1)
+
+test "$out" = "cache-corrupt: OK" || {
+    echo "expected 'cache-corrupt: OK', got: $out" >&2
+    exit 1
+}

tests/Makefile.am

@@ -64,6 +64,7 @@ TESTS = \
 	01_engine-useragent.test \
 	01_zlib-acceptencoding.test \
 	01_zlib-cache.test \
+	01_zlib-cache-corrupt.test \
 	01_zlib-cache-golden.test \
 	01_zlib-cache-writefail.test \
 	01_zlib-savename-cached.test \