tests: pin cdispo reserved-char sanitization in the savename table

Review follow-up: adds a row where the expected name differs from the
Content-Disposition input, exercising the hostile-cdispo cleaning path.

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

configure.ac

@@ -63,6 +63,16 @@ AC_SUBST(LT_CV_OBJDIR,$lt_cv_objdir)
 # Export version info
 AC_SUBST(VERSION_INFO)
 
+# Versioned plugin name for dlopen() in hts_create_opt(); soname major is
+# libtool's current - age, so this tracks VERSION_INFO bumps automatically.
+HTS_SONAME_MAJOR=$((${VERSION_INFO%%:*} - ${VERSION_INFO##*:}))
+case "$host_os" in
+darwin*) HTS_LIBHTSJAVA_NAME="libhtsjava.$HTS_SONAME_MAJOR.dylib" ;;
+*) HTS_LIBHTSJAVA_NAME="libhtsjava.so.$HTS_SONAME_MAJOR" ;;
+esac
+AC_DEFINE_UNQUOTED([HTS_LIBHTSJAVA_NAME], ["$HTS_LIBHTSJAVA_NAME"],
+  [Versioned libhtsjava runtime name, derived from VERSION_INFO])
+
 ### Default CFLAGS
 DEFAULT_CFLAGS="-Wall -Wformat -Wformat-security \
 -Wmultichar -Wwrite-strings -Wcast-qual -Wcast-align \

src/htscatchurl.c

@@ -175,7 +175,9 @@ HTSEXT_API hts_boolean catch_url(T_SOC soc, char *url, char *method,
       //
       socinput(soc, line, 1000);
       if (strnotempty(line)) {
-        if (sscanf(line, "%s %s %s", method, url, protocol) == 3) {
+        /* widths bound the caller buffers: method[32], url[HTS_URLMAXSIZE*2],
+           protocol[256] */
+        if (sscanf(line, "%31s %2047s %255s", method, url, protocol) == 3) {
           lien_adrfil af;
 
           // méthode en majuscule

src/htsdefines.h

@@ -69,11 +69,15 @@ typedef struct t_hts_callbackarg t_hts_callbackarg;
 typedef struct t_hts_callbackarg t_hts_callbackarg;
 #endif
 
-/* Marks a symbol an external wrapper module exports back to the engine
-   (dllexport on Windows, nothing elsewhere). */
+/* Marks a symbol an external wrapper module exports back to the engine.
+   Must override -fvisibility=hidden on ELF, or dlopen()ed plugins (htsjava)
+   hide their own hts_plug()/hts_unplug() entry points. */
 #ifndef EXTERNAL_FUNCTION
 #ifdef _WIN32
 #define EXTERNAL_FUNCTION __declspec(dllexport)
+#elif ((defined(__GNUC__) && (__GNUC__ >= 4)) ||                               \
+       (defined(HAVE_VISIBILITY) && HAVE_VISIBILITY))
+#define EXTERNAL_FUNCTION __attribute__((visibility("default")))
 #else
 #define EXTERNAL_FUNCTION
 #endif

src/htsencoding.c

@@ -190,9 +190,9 @@ int hts_unescapeEntitiesWithCharset(const char *src, char *dest, const size_t ma
         }
       }
     }
-    
-    /* copy */
-    if (j + 1 > max) {
+
+    /* reserve one byte for the trailing NUL written after the loop */
+    if (j + 1 >= max) {
       /* overflow */
       return -1;
     }
@@ -300,6 +300,11 @@ int hts_unescapeUrlSpecial(const char *src, char *dest, const size_t max,
 
           /* Was the character read successfully ? */
           if (nRead == utfBufferSize) {
+            /* the 'continue' below skips the NUL-reserve guard: re-check */
+            if (utfBufferJ + utfBufferSize >= max) {
+              return -1;
+            }
+
             /* Rollback write position to sequence start write position */
             j = utfBufferJ;
 
@@ -314,8 +319,8 @@ int hts_unescapeUrlSpecial(const char *src, char *dest, const size_t max,
       }
     }
 
-    /* Check for overflow */
-    if (j + 1 > max) {
+    /* reserve one byte for the trailing NUL written after the loop */
+    if (j + 1 >= max) {
       return -1;
     }
 

src/htsftp.c

@@ -133,6 +133,8 @@ void ftp_split_userpass(const char *src, const char *end, char *user,
                         size_t user_size, char *pass, size_t pass_size) {
   size_t n = 0;
 
+  assertf(user_size > 0 && pass_size > 0); /* the size-1 math underflows on 0 */
+
   while (src[n] != '\0' && src[n] != ':') {
     if (n < user_size - 1)
       user[n] = src[n];

src/htsftp.h

@@ -71,7 +71,8 @@ int run_launch_ftp(FTPDownloadStruct * params);
 int send_line(T_SOC soc, const char *data);
 int get_ftp_line(T_SOC soc, char *line, size_t line_size, int timeout);
 /* Split a "user[:pass]@" prefix (end = jump_identification result) into
-   bounded, NUL-terminated user/pass buffers, truncating to fit. */
+   bounded, NUL-terminated user/pass buffers, truncating to fit.
+   Both sizes must be nonzero. */
 void ftp_split_userpass(const char *src, const char *end, char *user,
                         size_t user_size, char *pass, size_t pass_size);
 T_SOC get_datasocket(char *to_send, size_t to_send_size);

src/htsjava.h

@@ -33,15 +33,19 @@ Please visit our Website: http://www.httrack.com
 #ifndef HTSJAVA_DEFH
 #define HTSJAVA_DEFH
 
+#include <stdint.h>
+
 #ifndef HTS_DEF_FWSTRUCT_JAVA_HEADER
 #define HTS_DEF_FWSTRUCT_JAVA_HEADER
 typedef struct JAVA_HEADER JAVA_HEADER;
 #endif
+/* 10-byte on-disk .class header image, fread() directly: fields need exact
+   widths (LP64's 8-byte 'unsigned long' magic never matched 0xCAFEBABE). */
 struct JAVA_HEADER {
-  unsigned long int magic;
-  unsigned short int minor;
-  unsigned short int major;
-  unsigned short int count;
+  uint32_t magic;
+  uint16_t minor;
+  uint16_t major;
+  uint16_t count;
 };
 
 #ifndef HTS_DEF_FWSTRUCT_RESP_STRUCT

src/htslib.c

@@ -1149,7 +1149,8 @@ int http_sendhead(httrackp * opt, t_cookie * cookie, int mode,
           char BIGSTK protocol[256], url[HTS_URLMAXSIZE * 2], method[256];
 
           linput(fp, line, 1000);
-          if (sscanf(line, "%s %s %s", method, url, protocol) == 3) {
+          /* widths bound method[256], url[HTS_URLMAXSIZE*2], protocol[256] */
+          if (sscanf(line, "%255s %2047s %255s", method, url, protocol) == 3) {
             size_t ret;
             // selon que l'on a ou pas un proxy
             if (retour->req.proxy.active) {
@@ -6022,9 +6023,11 @@ HTSEXT_API httrackp *hts_create_opt(void) {
     "htsswf", "htsjava", "httrack-plugin", NULL
   };
 #else
-  static const char *defaultModules[] = {
-    "libhtsswf.so.1", "libhtsjava.so.2", "httrack-plugin", NULL
-  };
+#ifndef HTS_LIBHTSJAVA_NAME
+#define HTS_LIBHTSJAVA_NAME "libhtsjava.so" /* non-autoconf fallback */
+#endif
+  static const char *defaultModules[] = {"libhtsswf.so.1", HTS_LIBHTSJAVA_NAME,
+                                         "httrack-plugin", NULL};
 #endif
   httrackp *opt = malloc(sizeof(httrackp));
 

src/htsname.c

@@ -167,8 +167,24 @@ static int wire_patches_ext(httrackp *opt, const char *wiremime,
   return 1;
 }
 
-// forme le nom du fichier à sauver (save) à partir de fil et adr
-// système intelligent, qui renomme en cas de besoin (exemple: deux INDEX.HTML et index.html)
+/* Wire-metadata name change: a Content-Disposition filename wins (returns 2),
+   else the declared type's ext when wire_patches_ext() allows (returns 1),
+   else 0. ext receives the new extension or replacement filename. */
+static int resolve_extension(httrackp *opt, const char *cdispo,
+                             const char *contenttype, const char *fil,
+                             char *ext, size_t ext_size) {
+  if (strnotempty(cdispo)) {
+    strlcpybuff(ext, cdispo, ext_size);
+    return 2;
+  }
+  if (wire_patches_ext(opt, contenttype, fil) &&
+      give_mimext(ext, ext_size, contenttype))
+    return 1;
+  return 0;
+}
+
+// Build the local save name (save) from adr/fil; renames on collision
+// (e.g. INDEX.HTML vs index.html).
 int url_savename(lien_adrfilsave *const afs,
                  lien_adrfil *const former,
                  const char *referer_adr, const char *referer_fil, 
@@ -405,45 +421,23 @@ int url_savename(lien_adrfilsave *const afs,
 
   // si option check_type activée
   if (is_html < 0 && opt->check_type && !ext_chg) {
-    int ishtest = 0;
-
     if (protocol != PROTOCOL_FILE
         && protocol != PROTOCOL_FTP
       ) {
       // tester type avec requète HEAD si on ne connait pas le type du fichier
       if (!((opt->check_type == 1) && (fil[strlen(fil) - 1] == '/')))   // slash doit être html?
         if (opt->savename_delayed == HTS_SAVENAME_DELAYED_HARD ||
-            (ishtest = ishtml(opt, fil)) <
-                0) { // unsure whether it's html or a file
+            ishtml(opt, fil) < 0) { // unsure whether it's html or a file
           // lire dans le cache
           htsblk r = cache_read_including_broken(opt, cache, adr, fil); // test uniquement
 
-          if (r.statuscode != -1) {     // pas d'erreur de lecture cache
-            char s[32];
-
-            s[0] = '\0';
+          if (r.statuscode != -1) { // cache entry read OK
             hts_log_print(opt, LOG_DEBUG, "Testing link type (from cache) %s%s",
                           adr_complete, fil_complete);
             if (!HTTP_IS_REDIRECT(r.statuscode)) {
-              if (strnotempty(r.cdispo)) {        /* filename given */
-                ext_chg = 2;      /* change filename */
-                strcpybuff(ext, r.cdispo);
-              } else if (wire_patches_ext(opt, r.contenttype, fil)) {
-                if (give_mimext(s, sizeof(s),
-                                r.contenttype)) { // recognized extension
-                  ext_chg = 1;
-                  strcpybuff(ext, s);
-                }
-              }
+              ext_chg = resolve_extension(opt, r.cdispo, r.contenttype, fil,
+                                          ext, sizeof(ext));
             }
-#ifdef DEFAULT_BIN_EXT
-            // no extension and potentially bogus
-            else if (ishtest == -2) {
-              ext_chg = 1;
-              strcpybuff(ext, DEFAULT_BIN_EXT + 1);
-            }
-#endif
-            //
           } else if (opt->savename_delayed != HTS_SAVENAME_DELAYED_HARD &&
                      is_userknowntype(opt, fil)) { /* PATCH BY BRIAN SCHRÖDER.
                               Lookup mimetype not only by extension,
@@ -467,22 +461,11 @@ int url_savename(lien_adrfilsave *const afs,
           // fail later
           else if (opt->savename_delayed != HTS_SAVENAME_DELAYED_NONE &&
                    !opt->state.stop) {
-            // Check if the file is ready in backing. We basically take the same logic as later.
-            // FIXME: we should cleanup and factorize this unholy mess
+            // Check if the file is ready in backing.
             if (headers != NULL && headers->status >= 0 && !is_redirect) {
-              if (strnotempty(headers->r.cdispo)) {        /* filename given */
-                ext_chg = 2;      /* change filename */
-                strcpybuff(ext, headers->r.cdispo);
-              } else if (wire_patches_ext(opt, headers->r.contenttype,
-                                          headers->url_fil)) {
-                char s[16];
-                if (give_mimext(
-                        s, sizeof(s),
-                        headers->r.contenttype)) { // recognized extension
-                  ext_chg = 1;
-                  strcpybuff(ext, s);
-                }
-              }
+              ext_chg = resolve_extension(opt, headers->r.cdispo,
+                                          headers->r.contenttype,
+                                          headers->url_fil, ext, sizeof(ext));
             }
             else if (mime_type != NULL) {
               ext[0] = '\0';
@@ -500,13 +483,6 @@ int url_savename(lien_adrfilsave *const afs,
                   if (!may_unknown2(opt, mime_type, fil)) {
                     ext_chg = 1;
                   }
-#ifdef DEFAULT_BIN_EXT
-                  // no extension and potentially bogus
-                  else if (ishtml(opt, fil) == -2) {
-                    ext_chg = 1;
-                    strcpybuff(ext, DEFAULT_BIN_EXT + 1);
-                  }
-#endif
                 } else {
                   ext_chg = 0;
                 }
@@ -696,30 +672,10 @@ int url_savename(lien_adrfilsave *const afs,
                     // libérer emplacement backing
                   }
 
-                  {             // pas d'erreur, changer type?
-                    char s[16];
-
-                    s[0] = '\0';
-                    if (strnotempty(back[b].r.cdispo)) {        /* filename given */
-                      ext_chg = 2;      /* change filename */
-                      strcpybuff(ext, back[b].r.cdispo);
-                    } else if (wire_patches_ext(opt, back[b].r.contenttype,
-                                                back[b].url_fil)) {
-                      if (give_mimext(
-                              s, sizeof(s),
-                              back[b].r.contenttype)) { // recognized extension
-                        ext_chg = 1;
-                        strcpybuff(ext, s);
-                      }
-                    }
-#ifdef DEFAULT_BIN_EXT
-                    // no extension and potentially bogus
-                    else if (ishtest == -2) {
-                      ext_chg = 1;
-                      strcpybuff(ext, DEFAULT_BIN_EXT + 1);
-                    }
-#endif
-                  }
+                  // no error: change the type?
+                  ext_chg = resolve_extension(
+                      opt, back[b].r.cdispo, back[b].r.contenttype,
+                      back[b].url_fil, ext, sizeof(ext));
                 }
                 // FIN Si non déplacé, forcer type?
 

src/htsparse.c

@@ -604,13 +604,14 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                         }
 
                         // Decode title with encoding
-                        if (str->page_charset_ != NULL 
-                            && *str->page_charset_ != '\0') {
-                          char *const sUtf = 
-                            hts_convertStringToUTF8(s, strlen(s), str->page_charset_);
+                        if (str->page_charset_ != NULL &&
+                            *str->page_charset_ != '\0') {
+                          char *sUtf = hts_convertStringToUTF8(
+                              s, strlen(s), str->page_charset_);
                           if (sUtf != NULL) {
-                            strcpy(s, sUtf);
-                            free(sUtf);
+                            /* UTF-8 can expand past s[]; truncate to fit */
+                            snprintf(s, sizeof(s), "%s", sUtf);
+                            freet(sUtf);
                           }
                         }
 

src/htsselftest.c

@@ -713,7 +713,8 @@ static int st_entities(httrackp *opt, int argc, char **argv) {
   }
   s = strdupt(argv[0]);
   enc = argc >= 2 ? argv[1] : "UTF-8";
-  if (s != NULL && hts_unescapeEntitiesWithCharset(s, s, strlen(s), enc) == 0) {
+  if (s != NULL &&
+      hts_unescapeEntitiesWithCharset(s, s, strlen(s) + 1, enc) == 0) {
     printf("%s\n", s);
     freet(s);
   } else {
@@ -722,6 +723,34 @@ static int st_entities(httrackp *opt, int argc, char **argv) {
   return 0;
 }
 
+/* The unescapers must reserve one byte for the trailing NUL: a 'max'-byte
+   dest holding 'max' output chars pre-fix wrote dest[max] (1-byte OOB, caught
+   by ASan). Both unescapeEntities and unescapeUrl share the guard. */
+static int st_unescape_bounds(httrackp *opt, int argc, char **argv) {
+  char dest[4];
+
+  (void) opt;
+  (void) argc;
+  (void) argv;
+  assertf(hts_unescapeEntities("abcd", dest, sizeof(dest)) == -1);
+  assertf(hts_unescapeUrl("abcd", dest, sizeof(dest)) == -1);
+  assertf(hts_unescapeEntities("abc", dest, sizeof(dest)) == 0);
+  assertf(strcmp(dest, "abc") == 0);
+  /* raw multi-byte UTF-8 flush path (bypasses the per-byte guard) */
+  assertf(hts_unescapeUrl("ab\xC3\xA9", dest, sizeof(dest)) == -1);
+  assertf(hts_unescapeUrl("a\xC3\xA9", dest, sizeof(dest)) == 0);
+  assertf(strcmp(dest, "a\xC3\xA9") == 0);
+  {
+    /* %xx-encoded flush path (utfBufferJ = lastJ rollback) */
+    char wide[8];
+
+    assertf(hts_unescapeUrl("%C3%A9", wide, sizeof(wide)) == 0);
+    assertf(strcmp(wide, "\xC3\xA9") == 0);
+  }
+  printf("unescape-bounds self-test OK\n");
+  return 0;
+}
+
 static int st_hashtable(httrackp *opt, int argc, char **argv) {
   char *snum;
   unsigned long count = 0;
@@ -1089,6 +1118,8 @@ static int st_savename(httrackp *opt, int argc, char **argv) {
   headers.status = 0;
   headers.r.statuscode = HTTP_OK;
   strcpybuff(headers.r.contenttype, argv[1]);
+  if (argc >= 3)
+    strcpybuff(headers.r.cdispo, argv[2]);
   strcpybuff(headers.url_fil, argv[0]);
 
   url_savename(&afs, NULL, NULL, NULL, opt, sback, &cache, &hash, 0, 0,
@@ -1824,6 +1855,17 @@ static int st_ftpuser(httrackp *opt, int argc, char **argv) {
   ftp_split_userpass(in, in + 802, user, sizeof(user), pass, sizeof(pass));
   assertf(strlen(user) == sizeof(user) - 1);
   assertf(strlen(pass) == sizeof(pass) - 1);
+  {
+    /* tight sizes + guard byte catch an off-by-one the 256 case can't */
+    char ubuf[16], pbuf[16];
+
+    memset(ubuf, 'Z', sizeof(ubuf));
+    memset(pbuf, 'Z', sizeof(pbuf));
+    ftp_split_userpass(in, in + 802, ubuf, 8, pbuf, 8);
+    assertf(strcmp(ubuf, "uuuuuuu") == 0);
+    assertf(strcmp(pbuf, "ppppppp") == 0);
+    assertf(ubuf[8] == 'Z' && pbuf[8] == 'Z');
+  }
   printf("ftp-userpass self-test OK\n");
   return 0;
 }
@@ -1873,6 +1915,8 @@ static const struct selftest_entry {
     {"idna-decode", "<host>", "decode an IDNA/punycode hostname",
      st_idna_decode},
     {"entities", "<string> [encoding]", "unescape HTML entities", st_entities},
+    {"unescape-bounds", "", "unescapers reserve the NUL byte (no 1-byte OOB)",
+     st_unescape_bounds},
     {"hashtable", "<count|file>", "coucal hashtable stress test", st_hashtable},
     {"strsafe", "[overflow|overflow-buff [str]]", "bounded string-op self-test",
      st_strsafe},
@@ -1882,7 +1926,7 @@ static const struct selftest_entry {
      st_relative},
     {"resolve", "<link> <adr> <fil>", "resolve a link against an origin",
      st_resolve},
-    {"savename", "<fil> <content-type>", "local save-name for a URL",
+    {"savename", "<fil> <content-type> [cdispo]", "local save-name for a URL",
      st_savename},
     {"cache", "<dir>", "cache read/write round-trip self-test", st_cache},
     {"cache-golden", "<dir> [regen]", "frozen cache-format read self-test",

tests/01_engine-savename.test

@@ -3,11 +3,11 @@
 
 set -euo pipefail
 
-# Local save-name extension resolution (url_savename via -#test=savename <fil> <content-type>).
+# Local save-name extension resolution (url_savename via -#test=savename <fil> <content-type> [cdispo]).
 # Asserts on the basename of "savename: <path>".
 
 name() {
-    out="$(httrack -O /dev/null -#test=savename "$1" "$2" | sed -n 's/^savename: //p')"
+    out="$(httrack -O /dev/null -#test=savename "$1" "$2" ${4:+"$4"} | sed -n 's/^savename: //p')"
     test "${out##*/}" == "$3" || {
         echo "FAIL: '$1' '$2' -> '$out' (want '$3')"
         exit 1
@@ -39,3 +39,10 @@ name '/types/data.json' 'application/json' 'data.json'
 
 # Agreeing type must not rewrite the extension's casing (no strip-and-reappend).
 name '/x.JPG' 'image/jpeg' 'x.JPG'
+
+# A Content-Disposition filename replaces the URL name outright.
+name '/x.php' 'application/pdf' 'report.pdf' 'report.pdf'
+name '/download' 'text/html' 'setup.exe' 'setup.exe'
+
+# Reserved characters in a hostile Content-Disposition name are sanitized.
+name '/x.php' 'application/pdf' 'set_up.exe' 'set:up.exe'

tests/01_engine-unescape-bounds.test

@@ -0,0 +1,7 @@
+#!/bin/bash
+#
+
+set -euo pipefail
+
+# Entity/URL unescapers reserve one byte for the trailing NUL (no 1-byte OOB).
+httrack -O /dev/null -#test=unescape-bounds run | grep -q "unescape-bounds self-test OK"

tests/31_local-javaclass.test

@@ -0,0 +1,23 @@
+#!/bin/bash
+# The java plugin must load (versioned dlopen name) and parse a .class
+# constant pool: a resource named only inside Foo.class gets crawled.
+set -e
+
+: "${top_srcdir:=..}"
+
+tmproot=$(mktemp -d)
+trap 'rm -rf "$tmproot"' EXIT
+mkdir "$tmproot/javaclass"
+
+cat >"$tmproot/javaclass/index.html" <<'EOF'
+<html><body><a href="Foo.class">applet</a></body></html>
+EOF
+printf 'GIF89a' >"$tmproot/javaclass/hello.gif"
+# magic/minor/major, count=2, one CONSTANT_Utf8 "hello.gif", class/superclass
+printf '\xCA\xFE\xBA\xBE\x00\x00\x00\x32\x00\x02\x01\x00\x09hello.gif\x00\x00\x00\x00' \
+    >"$tmproot/javaclass/Foo.class"
+
+bash "$top_srcdir/tests/local-crawl.sh" --root "$tmproot" --errors 0 \
+    --found 'javaclass/Foo.class' \
+    --found 'javaclass/hello.gif' \
+    httrack 'BASEURL/javaclass/index.html'

tests/Makefile.am

@@ -57,6 +57,7 @@ TESTS = \
 	01_engine-stripquery.test \
 	01_engine-strsafe.test \
 	01_engine-urlhack.test \
+	01_engine-unescape-bounds.test \
 	01_engine-useragent.test \
 	01_zlib-acceptencoding.test \
 	01_zlib-cache.test \
@@ -89,6 +90,7 @@ TESTS = \
 	27_local-cookies-file.test \
 	28_local-pause.test \
 	29_local-redirect-fragment.test \
-	30_local-fragment-link.test
+	30_local-fragment-link.test \
+	31_local-javaclass.test
 
 CLEANFILES = check-network_sh.cache