Allocate exactly one extra byte for cache-buffer NUL terminators

These fread buffers were over-allocated as size+4, a superstitious margin
that never bought anything: every site writes a single trailing NUL at
[size], so size+1 is exactly right. Trim them all to size+1.

The proxytrack disk-fallback read in PT_ReadCache__New_u never wrote that
NUL at all, unlike its sibling read paths in the same file; add the missing
r->adr[r->size] = '\0' so the spare byte is actually used and the buffer is
a valid C string.

Signed-off-by: Xavier Roche <roche@httrack.com>

.github/workflows/ci.yml

@@ -133,6 +133,97 @@ jobs:
         if: failure()
         run: cat tests/test-suite.log 2>/dev/null || true
 
+  # Memory safety: build and run the suite under AddressSanitizer +
+  # UndefinedBehaviorSanitizer. The offline engine self-tests drive the parsers
+  # that chew on untrusted crawled input (charset, mime, HTML, entities, IDNA,
+  # filters, cache) straight through the sanitizers, so a buffer overrun,
+  # use-after-free, or signed overflow there fails the build instead of slipping
+  # past a plain -O2 build. gcc's runtimes; one job is enough (the bug class is
+  # arch-independent and the matrix already covers compile portability).
+  sanitize:
+    name: sanitize (ASan+UBSan, gcc)
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          submodules: recursive
+
+      - name: Install build dependencies
+        run: |
+          set -euo pipefail
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            build-essential autoconf automake libtool autoconf-archive \
+            zlib1g-dev libssl-dev
+
+      - name: Configure (sanitized)
+        run: |
+          set -euo pipefail
+          autoreconf -fi
+          ./configure CC=gcc \
+            CFLAGS="-fsanitize=address,undefined -fno-sanitize-recover=all -g -O1 -fno-omit-frame-pointer" \
+            LDFLAGS="-fsanitize=address,undefined"
+
+      - name: Build
+        run: make -j"$(nproc)"
+
+      - name: Test (sanitized)
+        # Leaks at exit are out of scope (the CLI frees little on the way out);
+        # we want memory-safety errors, so turn leak detection off and make every
+        # other finding abort the run.
+        #
+        # Poison fresh allocations with 0xCA and freed blocks with 0xCB (decimal
+        # 202/203) so memory never reads back as accidental zeros: a missing-NUL
+        # fread buffer then runs strlen off into the redzone instead of stopping
+        # at a lucky zero. Distinct bytes tell the two apart in a dump (0xCA =
+        # uninitialized, 0xCB = use-after-free). ASan caps its malloc fill at 4096
+        # bytes by default, so max_malloc_fill_size lifts it to cover large cache
+        # buffers; free_fill flags use-after-free reads.
+        env:
+          ASAN_OPTIONS: detect_leaks=0:abort_on_error=1:halt_on_error=1:strict_string_checks=1:malloc_fill_byte=202:max_malloc_fill_size=2147483647:free_fill_byte=203:max_free_fill_size=2147483647
+          UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
+        run: make check
+
+      - name: Print the test log on failure
+        if: failure()
+        run: cat tests/test-suite.log 2>/dev/null || true
+
+  # Optional-dependency build: compile and test with HTTPS/OpenSSL disabled --
+  # the configuration users on minimal systems build, and one libssl is not even
+  # installed here so configure cannot silently re-enable it. The matrix above
+  # always has libssl, so the #if HTS_USEOPENSSL branches would otherwise never
+  # be compiled and could rot unnoticed.
+  no-ssl:
+    name: build (no openssl, --disable-https)
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          submodules: recursive
+
+      - name: Install build dependencies (no libssl)
+        run: |
+          set -euo pipefail
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            build-essential autoconf automake libtool autoconf-archive zlib1g-dev
+
+      - name: Configure (https disabled)
+        run: |
+          set -euo pipefail
+          autoreconf -fi
+          ./configure --disable-https
+
+      - name: Build
+        run: make -j"$(nproc)"
+
+      - name: Test
+        run: make check
+
+      - name: Print the test log on failure
+        if: failure()
+        run: cat tests/test-suite.log 2>/dev/null || true
+
   # Validate the Debian packaging via the same script maintainers release with.
   # One amd64/gcc run is enough: packaging (control/rules/manifest/lintian/quilt
   # source build) is arch- and compiler-independent, and the build matrix above
@@ -167,6 +258,34 @@ jobs:
           export DEB_BUILD_OPTIONS="noautodbgsym parallel=$(nproc)"
           bash tools/mkdeb.sh --unsigned --no-release-artifacts
 
+  # Release-tarball integrity: `make distcheck` rolls the dist tarball, then
+  # configures, builds and tests it out-of-tree from a read-only source tree and
+  # checks nothing is left behind. Catches a file referenced in *_SOURCES or
+  # EXTRA_DIST but missing from the tarball -- the same "ships broken to users"
+  # class as a stale committed Makefile.in.
+  distcheck:
+    name: distcheck (release tarball)
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          submodules: recursive
+
+      - name: Install build dependencies
+        run: |
+          set -euo pipefail
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            build-essential autoconf automake libtool autoconf-archive \
+            zlib1g-dev libssl-dev
+
+      - name: distcheck
+        run: |
+          set -euo pipefail
+          autoreconf -fi
+          ./configure
+          make -j"$(nproc)" distcheck
+
   dco:
     name: DCO sign-off
     # Only checkable on a PR, where we have the base..head commit range.

src/Makefile.am

@@ -114,5 +114,12 @@ EXTRA_DIST = httrack.h webhttrack \
 		proxy/proxytrack.h \
 		proxy/store.h \
 		proxy/proxytrack.vcproj \
-		coucal/* \
-		*.dsw *.dsp *.vcproj
+		coucal/LICENSE \
+		coucal/Makefile \
+		coucal/README.md \
+		coucal/sample.c \
+		coucal/tests.c \
+		htsjava.vcproj \
+		httrack.dsp httrack.dsw httrack.vcproj \
+		libhttrack.dsp libhttrack.dsw libhttrack.vcproj \
+		webhttrack.dsp webhttrack.dsw webhttrack.vcproj

src/Makefile.in

@@ -565,8 +565,15 @@ EXTRA_DIST = httrack.h webhttrack \
 		proxy/proxytrack.h \
 		proxy/store.h \
 		proxy/proxytrack.vcproj \
-		coucal/* \
-		*.dsw *.dsp *.vcproj
+		coucal/LICENSE \
+		coucal/Makefile \
+		coucal/README.md \
+		coucal/sample.c \
+		coucal/tests.c \
+		htsjava.vcproj \
+		httrack.dsp httrack.dsw httrack.vcproj \
+		libhttrack.dsp libhttrack.dsw libhttrack.vcproj \
+		webhttrack.dsp webhttrack.dsw webhttrack.vcproj
 
 all: all-am
 

src/htscache.c

@@ -939,7 +939,7 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
                   FILE *const fp = FOPEN(fconv(catbuff, sizeof(catbuff), previous_save), "rb");
 
                   if (fp != NULL) {
-                    r.adr = (char *) malloct((int) r.size + 4);
+                    r.adr = (char *) malloct((int) r.size + 1);
                     if (r.adr != NULL) {
                       if (r.size > 0
                           && fread(r.adr, 1, (int) r.size, fp) != r.size) {
@@ -948,7 +948,8 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
                         r.statuscode = STATUSCODE_INVALID;
                         sprintf(r.msg, "Read error in cache disk data: %s",
                                 strerror(last_errno));
-                      }
+                      } else if (r.size >= 0)
+                        *(r.adr + r.size) = '\0';
                     } else {
                       r.statuscode = STATUSCODE_INVALID;
                       strcpybuff(r.msg,
@@ -965,7 +966,7 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
               // Data in cache.
               else {
                 // lire fichier (d'un coup)
-                r.adr = (char *) malloct((int) r.size + 4);
+                r.adr = (char *) malloct((int) r.size + 1);
                 if (r.adr != NULL) {
                   if (unzReadCurrentFile((unzFile) cache->zipInput, r.adr, (int) r.size) != r.size) {   // erreur
                     freet(r.adr);
@@ -1245,13 +1246,14 @@ static htsblk cache_readex_old(httrackp * opt, cache_back * cache,
                     FILE *fp = FOPEN(fconv(catbuff, sizeof(catbuff), return_save), "rb");
 
                     if (fp != NULL) {
-                      r.adr = (char *) malloct((size_t) r.size + 4);
+                      r.adr = (char *) malloct((size_t) r.size + 1);
                       if (r.adr != NULL) {
                         if (r.size > 0
                             && fread(r.adr, 1, (size_t) r.size, fp) != r.size) {
                           r.statuscode = STATUSCODE_INVALID;
                           strcpybuff(r.msg, "Read error in cache disk data");
-                        }
+                        } else if (r.size >= 0)
+                          *(r.adr + r.size) = '\0';
                       } else {
                         r.statuscode = STATUSCODE_INVALID;
                         strcpybuff(r.msg,
@@ -1266,7 +1268,7 @@ static htsblk cache_readex_old(httrackp * opt, cache_back * cache,
                 }
               } else {
                 // lire fichier (d'un coup)
-                r.adr = (char *) malloct((size_t) r.size + 4);
+                r.adr = (char *) malloct((size_t) r.size + 1);
                 if (r.adr != NULL) {
                   if (fread(r.adr, 1, (size_t) r.size, cache->olddat) != r.size) {      // erreur
                     freet(r.adr);
@@ -1369,10 +1371,11 @@ int cache_readdata(cache_back * cache, const char *str1, const char *str2,
 
         cache_rint(cache->olddat, &len);
         if (len > 0) {
-          char *mem_buff = (char *) malloct(len + 4);   /* Plus byte 0 */
+          char *mem_buff = (char *) malloct(len + 1); /* trailing \0 */
 
           if (mem_buff) {
             if (fread(mem_buff, 1, len, cache->olddat) == len) {        // lire tout (y compris statuscode etc)*/
+              mem_buff[len] = '\0';
               *inbuff = mem_buff;
               *inlen = len;
               return 1;

src/htscache_selftest.c

@@ -182,6 +182,16 @@ static int check_entry(httrackp *opt, cache_back *cache, const char *adr,
     fail++;
   }
 
+  /* The loaded body must be NUL-terminated at [size]: cache_readex's strlen()
+     consumers (htscore.c:1046, htscache.c) rely on it, and a missing
+     terminator is a heap over-read. The buffer is malloc(size + slack), so
+     reading [size] is in bounds. */
+  if (r.adr != NULL && r.adr[r.size] != '\0') {
+    fprintf(stderr, "cache-selftest: %s%s: body not NUL-terminated at [size]\n",
+            adr, fil);
+    fail++;
+  }
+
 #undef CHECK_STR
 
   if (r.adr != NULL) {
@@ -208,6 +218,107 @@ static void gen_body(char *buf, size_t len, int kind) {
   }
 }
 
+/* Exercise the disk-fallback read path: a record stored with X-In-Cache: 0
+   keeps its body on disk (not in the ZIP), and cache_readex must load it from
+   there. The one-shot crawl tests never re-read such a body into memory, so
+   this path otherwise has no runtime coverage. We store the header with
+   all_in_cache=0 and a non-hypertext content-type (-> X-In-Cache: 0), create
+   the body at the exact fconv()-resolved path the reader uses, then read it
+   back and assert it round-trips and is NUL-terminated. */
+static int disk_fallback_selftest(httrackp *opt) {
+  int fail = 0;
+  cache_back cache;
+  htsblk r;
+  char catbuff[HTS_URLMAXSIZE * 2];
+  char *path;
+  char *locbuf;
+  FILE *fp;
+  const char *const adr = "example.com";
+  const char *const fil = "/blob.bin";
+  char save[HTS_URLMAXSIZE * 2];
+  /* no embedded NUL: were the read to leave this un-terminated, a later
+     strlen() would run off the end (the bug this guards) */
+  static const char body[] = "BINARY-on-disk-body-0123456789-no-trailing-nul";
+  const size_t body_len = sizeof(body) - 1;
+
+  /* X-Save must start with path_html_utf8 so the reader resolves it verbatim
+     (otherwise it re-roots it as a pre-3.40 relative path); then the body we
+     create at fconv(save) is exactly where cache_readex looks for it. */
+  fconcat(save, sizeof(save), StringBuff(opt->path_html_utf8),
+          "example.com/blob.bin");
+
+  /* write only the header (X-In-Cache: 0); the body stays on disk */
+  selftest_open_for_write(&cache, opt);
+  {
+    htsblk w;
+    char locw[4];
+    char *bodycopy = malloct(body_len);
+
+    hts_init_htsblk(&w);
+    w.statuscode = 200;
+    w.size = (LLint) body_len;
+    strcpybuff(w.msg, "OK");
+    strcpybuff(w.contenttype, "application/octet-stream");
+    locw[0] = '\0';
+    w.location = locw;
+    w.is_write = 0;
+    memcpy(bodycopy, body, body_len);
+    w.adr = bodycopy;
+    cache_add(opt, &cache, &w, adr, fil, save, 0 /* all_in_cache */, NULL);
+    freet(bodycopy);
+  }
+  selftest_close(&cache);
+
+  /* create the on-disk body where the reader will look for it */
+  path = fconv(catbuff, sizeof(catbuff), save);
+  (void) structcheck(path);
+  fp = FOPEN(path, "wb");
+  if (fp == NULL) {
+    fprintf(stderr, "cache-selftest: disk-fallback: cannot create '%s'\n",
+            path);
+    return 1;
+  }
+  if (fwrite(body, 1, body_len, fp) != body_len) {
+    fprintf(stderr, "cache-selftest: disk-fallback: short write to '%s'\n",
+            path);
+    fail++;
+  }
+  fclose(fp);
+
+  /* read it back: takes the X-In-Cache: 0 disk-fallback branch */
+  selftest_open_for_read(&cache, opt);
+  locbuf = malloct(HTS_URLMAXSIZE * 2);
+  locbuf[0] = '\0';
+  r = cache_readex(opt, &cache, adr, fil, "", locbuf, NULL, 1);
+  if (r.statuscode != 200) {
+    fprintf(stderr,
+            "cache-selftest: disk-fallback: statuscode %d, expected 200"
+            " (path not taken or read failed)\n",
+            r.statuscode);
+    fail++;
+  }
+  if (r.size != (LLint) body_len) {
+    fprintf(stderr,
+            "cache-selftest: disk-fallback: size " LLintP ", expected %d\n",
+            (LLint) r.size, (int) body_len);
+    fail++;
+  } else if (r.adr == NULL || memcmp(r.adr, body, body_len) != 0) {
+    fprintf(stderr, "cache-selftest: disk-fallback: body mismatch\n");
+    fail++;
+  }
+  /* the loaded body must be NUL-terminated at [size] */
+  if (r.adr != NULL && r.adr[r.size] != '\0') {
+    fprintf(stderr, "cache-selftest: disk-fallback: body not NUL-terminated\n");
+    fail++;
+  }
+  if (r.adr != NULL) {
+    freet(r.adr);
+  }
+  freet(locbuf);
+  selftest_close(&cache);
+  return fail;
+}
+
 int cache_selftests(httrackp *opt, const char *dir) {
   int failures = 0;
   cache_back cache;
@@ -257,6 +368,10 @@ int cache_selftests(httrackp *opt, const char *dir) {
       strcatbuff(base, "/");
     }
     StringCopy(opt->path_log, base);
+    /* the disk-fallback pass resolves on-disk body paths through fconv(), which
+       is rooted at path_html; keep it inside the test directory too */
+    StringCopy(opt->path_html, base);
+    StringCopy(opt->path_html_utf8, base);
   }
   opt->cache = 1;
 
@@ -366,6 +481,9 @@ int cache_selftests(httrackp *opt, const char *dir) {
                   "", body_updated, strlen(body_updated));
   selftest_close(&cache);
 
+  /* pass 5: the disk-fallback read path (X-In-Cache: 0, body on disk) */
+  failures += disk_fallback_selftest(opt);
+
   for (i = 0; i < large_count; i++) {
     freet(large_body[i]);
   }

src/htscore.c

@@ -2193,16 +2193,19 @@ int httpmirror(char *url1, httrackp * opt) {
                 (OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), StringBuff(opt->path_log),
                  "hts-cache/new.lst"), "rb");
         if (new_lst != NULL && sz != (size_t) -1) {
-          char *adr = (char *) malloct(sz);
+          /* +1 for the NUL below: new.lst is read raw, and the strstr()
+             that follows needs a terminated C string. */
+          char *adr = (char *) malloct(sz + 1);
 
           if (adr) {
             if (fread(adr, 1, sz, new_lst) == sz) {
+              adr[sz] = '\0';
               char line[1100];
               int purge = 0;
 
               while(!feof(old_lst)) {
                 linput(old_lst, line, 1000);
-                if (!strstr(adr, line)) {       // fichier non trouvé dans le nouveau?
+                if (!strstr(adr, line)) { // not found in the new list?
                   char BIGSTK file[HTS_URLMAXSIZE * 2];
 
                   strcpybuff(file, StringBuff(opt->path_html));

src/htsencoding.c

@@ -145,8 +145,13 @@ int hts_unescapeEntitiesWithCharset(const char *src, char *dest, const size_t ma
         if (!hex) {
           if (src[i] >= '0' && src[i] <= '9') {
             const int h = src[i] - '0';
-            uc *= 10;
-            uc += h;
+            /* Guard before multiplying: a codepoint past the Unicode max
+               (0x10FFFF) is invalid anyway, so stop rather than overflow uc. */
+            if (uc > (0x10FFFF - h) / 10) {
+              ampStart = (size_t) -1;
+            } else {
+              uc = uc * 10 + h;
+            }
           } else {
             /* abandon */
             ampStart = (size_t) -1;
@@ -156,8 +161,11 @@ int hts_unescapeEntitiesWithCharset(const char *src, char *dest, const size_t ma
         else {
           const int h = get_hex_value(src[i]);
           if (h != -1) {
-            uc *= 16;
-            uc += h;
+            if (uc > (0x10FFFF - h) / 16) {
+              ampStart = (size_t) -1;
+            } else {
+              uc = uc * 16 + h;
+            }
           } else {
             /* abandon */
             ampStart = (size_t) -1;

src/htsindex.c

@@ -334,7 +334,7 @@ void index_finish(const char *indexpath, int mode) {
     if (fp_tmpproject) {
       tab = (char **) malloct(sizeof(char *) * (hts_primindex_size + 2));
       if (tab) {
-        blk = malloct(size + 4);
+        blk = malloct(size + 1);
         if (blk) {
           fseek(fp_tmpproject, 0, SEEK_SET);
           if ((INTsys) fread(blk, 1, size, fp_tmpproject) == size) {
@@ -343,6 +343,7 @@ void index_finish(const char *indexpath, int mode) {
             int i;
             FILE *fp;
 
+            blk[size] = '\0';
             while((b = strchr(a, '\n')) && (index < hts_primindex_size)) {
               tab[index++] = a;
               *b = '\0';

src/htsparse.c

@@ -3416,8 +3416,17 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
           if (RUN_CALLBACK4(opt, postprocess, &cAddr, &cSize, urladr(), urlfil()) == 1) {
             hts_log_print(opt, LOG_DEBUG,
               "engine: postprocess-html: callback modified data, applying %d bytes", cSize);
-            TypedArraySize(output_buffer) = 0;
-            TypedArrayAppend(output_buffer, cAddr, cSize);
+            /* The callback either edits output_buffer in place (cAddr
+               unchanged) or hands back its own buffer (cAddr changed). Only
+               the latter needs a copy: re-appending output_buffer onto itself
+               would read freed memory, as the append's realloc can relocate
+               the block out from under cAddr. */
+            if (cAddr != TypedArrayElts(output_buffer)) {
+              TypedArraySize(output_buffer) = 0;
+              TypedArrayAppend(output_buffer, cAddr, cSize);
+            } else {
+              TypedArraySize(output_buffer) = (size_t) cSize;
+            }
           }
         }
 

src/proxy/store.c

@@ -1162,7 +1162,7 @@ static PT_Element PT_ReadCache__New_u(PT_Index index_, const char *url,
                     FILE *fp = fopen(file_convert(catbuff, sizeof(catbuff), previous_save), "rb");
 
                     if (fp != NULL) {
-                      r->adr = (char *) malloc(r->size + 4);
+                      r->adr = (char *) malloc(r->size + 1);
                       if (r->adr != NULL) {
                         if (r->size > 0
                             && fread(r->adr, 1, r->size, fp) != r->size) {
@@ -1172,6 +1172,7 @@ static PT_Element PT_ReadCache__New_u(PT_Index index_, const char *url,
                           sprintf(r->msg, "Read error in cache disk data: %s",
                                   strerror(last_errno));
                         }
+                        r->adr[r->size] = '\0';
                       } else {
                         r->statuscode = STATUSCODE_INVALID;
                         strcpy(r->msg,