Signed-shift UB in zip-repair local-header read

unzRepair parses the local file header of a damaged cache zip during
repair. READ_32 combined two int-typed READ_16 halves as
READ_16(adr) | (READ_16((adr)+2) << 16); when the high half has bit 15
set (>= 0x8000), shifting it left by 16 exceeds INT_MAX, which is signed
overflow. UBSan aborts on the CRC/size fields of a header whose high
16-bit word has that bit set. Repair runs on hostile input (a corrupt or
foreign new.zip). Cast the halves to uLong before the shift, matching the
sibling minizip readers in unzip.c and zip.c.

Closes #639

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

src/htsparse.c

@@ -315,6 +315,43 @@ static void escape_url_parens(char *const s, const size_t size) {
   strlcpybuff(s, buff, size);
 }
 
+/* Strip a default ":80" from lien's authority in place. Any spelling that
+   range-parses to 80 (":80", ":080") is dropped by its matched length, not a
+   hardcoded 3 chars; a value that merely wraps to 80 as an int (#614) stays. */
+void hts_strip_default_port(char *lien, size_t size) {
+  char *a;
+
+  if (!link_has_authority(lien))
+    return;
+  a = strstr(lien, "//"); // "//" authority
+  if (a)
+    a += 2;
+  else
+    a = lien;
+  a = jump_toport(a);
+  if (a) { // port present
+    char *b = a + 1;
+    char saved;
+    int port;
+    hts_boolean is_default;
+
+    while (isdigit((unsigned char) *b))
+      b++;
+    saved = *b;
+    *b = '\0';
+    is_default = hts_parse_url_port(a + 1, &port) && port == 80;
+    *b = saved;
+    if (is_default) { // default port, strip it
+      char BIGSTK tempo[HTS_URLMAXSIZE * 2];
+
+      tempo[0] = '\0';
+      strncatbuff(tempo, lien, a - lien);
+      strcatbuff(tempo, b); // skip the whole matched :port
+      strlcpybuff(lien, tempo, size);
+    }
+  }
+}
+
 /* Main parser */
 int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
   char catbuff[CATBUFF_SIZE];
@@ -2138,38 +2175,8 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                     } while((b != a) && (b));
                   }
                 }
-                // éliminer les éventuels :80 (port par défaut!)
-                if (link_has_authority(lien)) {
-                  char *a;
-
-                  a = strstr(lien, "//");       // "//" authority
-                  if (a)
-                    a += 2;
-                  else
-                    a = lien;
-                  a = jump_toport(a);
-                  if (a) {      // port
-                    int port = 0;
-                    int defport = 80;
-                    char *b = a + 1;
-
-#if HTS_USEOPENSSL
-#endif
-                    while(isdigit((unsigned char) *b)) {
-                      port *= 10;
-                      port += (int) (*b - '0');
-                      b++;
-                    }
-                    if (port == defport) {      // port 80, default - c'est débile
-                      char BIGSTK tempo[HTS_URLMAXSIZE * 2];
-
-                      tempo[0] = '\0';
-                      strncatbuff(tempo, lien, a - lien);
-                      strcatbuff(tempo, a + 3); // sauter :80
-                      strcpybuff(lien, tempo);
-                    }
-                  }
-                }
+                // drop a default :80 port from the authority
+                hts_strip_default_port(lien, sizeof(lien));
                 // filtrer les parazites (mailto & cie)
                 /*
                    if (strfield(lien,"mailto:")) {  // ne pas traiter

src/htsparse.h

@@ -106,6 +106,10 @@ struct htsmoduleStructExtended {
 */
 int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre);
 
+/* Strip a default ":80" (any spelling) from an absolute link's authority, in
+   place into a buffer of the given size. */
+void hts_strip_default_port(char *lien, size_t size);
+
 /*
   Check for 301,302.. errors ("moved") and handle them; re-isuue requests, make
   rediretc file, handle filters considerations..

src/htsselftest.c

@@ -1589,6 +1589,41 @@ static int st_identabs(httrackp *opt, int argc, char **argv) {
   return 0;
 }
 
+/* Default-port strip (#627): a genuine 80 (any spelling) is removed by its
+   matched length, host preserved; a non-80 port or one that only wraps to 80 as
+   a 32-bit int (#614) is left intact. Guards the old bug where ":080"/":0080"
+   dropped a hardcoded 3 chars and glued the leftover digits onto the host. */
+static int st_stripport(httrackp *opt, int argc, char **argv) {
+  static const struct {
+    const char *in, *out;
+  } cases[] = {
+      {"http://127.0.0.1:80/x", "http://127.0.0.1/x"},
+      {"http://127.0.0.1:080/x", "http://127.0.0.1/x"},
+      {"http://127.0.0.1:0080/x", "http://127.0.0.1/x"},
+      {"http://127.0.0.1:80", "http://127.0.0.1"},
+      {"http://127.0.0.1:0081/x", "http://127.0.0.1:0081/x"},
+      {"http://127.0.0.1:81/x", "http://127.0.0.1:81/x"},
+      {"http://127.0.0.1:8080/x", "http://127.0.0.1:8080/x"},
+      {"http://127.0.0.1:4294967376/x", "http://127.0.0.1:4294967376/x"},
+      {"http://127.0.0.1/x", "http://127.0.0.1/x"},
+  };
+
+  size_t k;
+
+  (void) opt;
+  (void) argc;
+  (void) argv;
+  for (k = 0; k < sizeof(cases) / sizeof(cases[0]); k++) {
+    char BIGSTK buff[HTS_URLMAXSIZE * 2];
+
+    strcpybuff(buff, cases[k].in);
+    hts_strip_default_port(buff, sizeof(buff));
+    assertf(strcmp(buff, cases[k].out) == 0);
+  }
+  printf("stripport self-test OK\n");
+  return 0;
+}
+
 /* Extra args are key=value: adr= cdispo= statuscode= status= strip= urlhack=
    no-www= no-slash= no-query= n83= type=, plus repeatable prior=adr|fil|sav
    registering an already-crawled link (dedup/collision paths). */
@@ -2111,6 +2146,52 @@ static int st_cache_corrupt(httrackp *opt, int argc, char **argv) {
   return err;
 }
 
+/* Drives unzRepair over a damaged local file header whose CRC field's high
+   16-bit word has bit 15 set. Before the READ_32 fix that shifted an int and
+   overflowed, so UBSan aborts here; after it, repair recovers the one entry. */
+static int st_zip_repair_shift(httrackp *opt, int argc, char **argv) {
+  static const unsigned char zip[] = {
+      0x50, 0x4b, 0x03, 0x04, /* local file header signature */
+      0x14, 0x00,             /* version needed */
+      0x00, 0x00,             /* general purpose flag */
+      0x00, 0x00,             /* method */
+      0x00, 0x00,             /* time */
+      0x00, 0x00,             /* date */
+      0x00, 0x00, 0xe8, 0x8a, /* crc: high word 0x8ae8, bit 15 set */
+      0x00, 0x00, 0x00, 0x00, /* compressed size */
+      0x00, 0x00, 0x00, 0x00, /* uncompressed size */
+      0x01, 0x00,             /* filename length */
+      0x00, 0x00,             /* extra field length */
+      0x61                    /* filename "a" */
+  };
+  char in[HTS_URLMAXSIZE], out[HTS_URLMAXSIZE], tmp[HTS_URLMAXSIZE];
+  uLong nrec = 0, bytes = 0;
+  FILE *fp;
+  int err;
+
+  (void) opt;
+  if (argc < 1) {
+    fprintf(stderr, "zip-repair-shift: needs a directory\n");
+    return 1;
+  }
+  snprintf(in, sizeof(in), "%s/damaged.zip", argv[0]);
+  snprintf(out, sizeof(out), "%s/repair.zip", argv[0]);
+  snprintf(tmp, sizeof(tmp), "%s/repair.tmp", argv[0]);
+  fp = fopen(in, "wb");
+  if (fp == NULL || fwrite(zip, 1, sizeof(zip), fp) != sizeof(zip)) {
+    if (fp != NULL)
+      fclose(fp);
+    fprintf(stderr, "zip-repair-shift: cannot write %s\n", in);
+    return 1;
+  }
+  fclose(fp);
+  err = unzRepair(in, out, tmp, &nrec, &bytes);
+  printf("zip-repair-shift: %s (recovered %lu entr%s)\n",
+         (err == Z_OK && nrec == 1) ? "OK" : "FAIL", (unsigned long) nrec,
+         nrec == 1 ? "y" : "ies");
+  return (err == Z_OK && nrec == 1) ? 0 : 1;
+}
+
 static int st_cache_legacy(httrackp *opt, int argc, char **argv) {
   int err;
 
@@ -3189,6 +3270,8 @@ static const struct selftest_entry {
      st_socks5},
     {"identabs", "", "ident_url_absolute one-byte fil[] overflow self-test",
      st_identabs},
+    {"stripport", "", "default :80 port strip preserves host (#627)",
+     st_stripport},
     {"header", "<raw-header-line> ...", "response header-line parsing",
      st_header},
     {"headerlong", "[header-name:]",
@@ -3216,6 +3299,9 @@ static const struct selftest_entry {
      st_cache_legacy},
     {"cache-corrupt", "<dir>", "cache read-side corruption self-test",
      st_cache_corrupt},
+    {"zip-repair-shift", "<dir>",
+     "cache zip-repair header read must not overflow a signed shift",
+     st_zip_repair_shift},
     {"dns", "", "DNS resolver/cache self-test", st_dns},
     {"dnstimeout", "", "a slow DNS resolve is bounded and holds no lock",
      st_dnstimeout},

src/minizip/mztools.c

@@ -14,7 +14,7 @@
 
 #define READ_8(adr)  ((unsigned char)*(adr))
 #define READ_16(adr) ( READ_8(adr) | (READ_8(adr+1) << 8) )
-#define READ_32(adr) ( READ_16(adr) | (READ_16((adr)+2) << 16) )
+#define READ_32(adr) ((uLong) READ_16(adr) | ((uLong) READ_16((adr) + 2) << 16))
 
 #define WRITE_8(buff, n) do { \
   *((unsigned char*)(buff)) = (unsigned char) ((n) & 0xff); \

tests/01_engine-xfread.test

@@ -9,21 +9,23 @@ set -euo pipefail
 # drives both refusal paths and the accept path.
 
 out="$(httrack -O /dev/null -#test=xfread-limit)"
+# Match with here-strings, not `echo | grep -q`: under pipefail the early grep
+# exit SIGPIPEs echo and fails the pipeline even when the pattern matched.
 for case in bylen bygrow; do
-    echo "$out" | grep -q "${case}: refused=1 adr=null msg=In-memory content too large" || {
+    grep -q "${case}: refused=1 adr=null msg=In-memory content too large" <<<"$out" || {
         echo "FAIL ${case}: $out"
         exit 1
     }
 done
 
 # Exactly INT32_MAX must be refused too (the reallocs add 1).
-echo "$out" | grep -q 'boundary: msg=In-memory content too large' || {
+grep -q 'boundary: msg=In-memory content too large' <<<"$out" || {
     echo "FAIL boundary: $out"
     exit 1
 }
 
 # The guard must NOT fire for a legitimate small size.
-if echo "$out" | grep -q 'accept: msg=In-memory content too large'; then
+if grep -q 'accept: msg=In-memory content too large' <<<"$out"; then
     echo "FAIL accept (guard fired on a legit size): $out"
     exit 1
 fi

tests/01_zlib-repair-shift.test

@@ -0,0 +1,21 @@
+#!/bin/bash
+#
+# Keep this POSIX-portable: the harness runs it via $(BASH), which is a plain
+# POSIX /bin/sh on some platforms (e.g. macOS), so avoid bashisms and GNU-only
+# tool flags despite the #!/bin/bash above.
+
+# unzRepair header read must not overflow a signed shift (-#test=zip-repair-shift
+# <dir>). A damaged local file header whose CRC high word has bit 15 set made
+# READ_32 shift an int past INT_MAX; UBSan aborts before the fix casts to uLong.
+
+set -eu
+
+dir=$(mktemp -d)
+trap 'rm -rf "$dir"' EXIT
+
+out=$(httrack -#test=zip-repair-shift "$dir")
+
+printf '%s\n' "$out" | grep -qx "zip-repair-shift: OK (recovered 1 entry)" || {
+    echo "expected 'zip-repair-shift: OK (recovered 1 entry)', got: $out" >&2
+    exit 1
+}

tests/66_engine-port80-strip.test

@@ -0,0 +1,11 @@
+#!/bin/bash
+#
+
+set -euo pipefail
+
+# Stripping a default :80 from a crawled link used to skip a hardcoded 3 chars
+# (#627): ":080"/":0080" lost only ":80" and glued the rest onto the host
+# (127.0.0.1:080/x -> 127.0.0.10/x), and a value wrapping to 80 as a 32-bit int
+# (#614) was stripped as if it were the default. All assertions live in the
+# engine self-test.
+httrack -O /dev/null -#test=stripport | grep -q "stripport self-test OK"

tests/Makefile.am

@@ -84,6 +84,7 @@ TESTS = \
 	01_zlib-cache-legacy.test \
 	01_zlib-cache-golden.test \
 	01_zlib-cache-writefail.test \
+	01_zlib-repair-shift.test \
 	01_zlib-savename-cached.test \
 	02_manpage-regen.test \
 	02_update-cache.test \
@@ -146,6 +147,7 @@ TESTS = \
 	62_lang-integrity.test \
 	63_webhttrack-home.test \
 	64_local-intl-outdir.test \
-	65_port-siblings.test
+	65_port-siblings.test \
+	66_engine-port80-strip.test
 
 CLEANFILES = check-network_sh.cache