Compare commits

..

4 Commits

Author SHA1 Message Date
Xavier Roche
a52a2b146c Add an offline update/cache regression test
Every crawl test runs httrack exactly once (crawl-test.sh), so the cache read /
update path (cache_readex) -- recently touched by the buffer-bounding work -- had
zero regression coverage: the cache was written but never read back.

Add tests/02_update-cache.test, a self-contained file:// two-pass test (no
network, always runs): mirror a local site, re-mirror it unchanged (the cache-
read pass must complete with no errors -- guards a crash/abort in cache_readex),
then change a source file and re-mirror (the update must pick up the new content
-- guards the update decision that reads the cached metadata).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-06-14 16:29:45 +02:00
Xavier Roche
226a38d3d0 Merge pull request #340 from xroche/cleanup/htscache-bounds
Bound htscache.c cache-field and save-name copies
2026-06-14 15:58:04 +02:00
Xavier Roche
1e463f65a5 Bound htscache.c cache-field and save-name copies
ZIP_READFIELD_STRING (the cached ZIP-header field reader) copied
attacker-influenced cache-file values into fixed htsblk fields with an unchecked
strcpybuff -- benign for the char[] fields, but r.location is a char* (degrades
to raw strcpy). Thread the destination size into the macro: sizeof(field) for
the array fields, HTS_URLMAXSIZE*2 for r.location (it points into a buffer of
that size, in both the caller-supplied and the location_default case).

Also bound cache_readex's return_save copy (its one non-NULL caller passes a
HTS_URLMAXSIZE*2 buffer), the exact-sized malloc copy in cache_rstr's default
path (strlen(defaultdata)+1), and replace the two strcpybuff(r.location, "")
clears with a direct r.location[0] = '\0'.

htscache.c pointer-destination warnings 6 -> 0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-06-14 15:43:04 +02:00
Xavier Roche
09ed9968cd Merge pull request #339 from xroche/cleanup/htsbauth-bounds
Bound htsbauth cookie/auth buffer writes
2026-06-14 15:32:37 +02:00
3 changed files with 91 additions and 19 deletions

View File

@@ -196,12 +196,13 @@ struct cache_back_zip_entry {
int compressionMethod;
};
#define ZIP_READFIELD_STRING(line, value, refline, refvalue) do { \
if (line[0] != '\0' && strfield2(line, refline)) { \
strcpybuff(refvalue, value); \
line[0] = '\0'; \
} \
} while(0)
#define ZIP_READFIELD_STRING(line, value, refline, refvalue, refvalue_size) \
do { \
if (line[0] != '\0' && strfield2(line, refline)) { \
strlcpybuff(refvalue, value, refvalue_size); \
line[0] = '\0'; \
} \
} while (0)
#define ZIP_READFIELD_INT(line, value, refline, refvalue) do { \
if (line[0] != '\0' && strfield2(line, refline)) { \
int intval = 0; \
@@ -643,7 +644,7 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
} else {
r.location = location_default;
}
strcpybuff(r.location, "");
r.location[0] = '\0';
strcpybuff(buff, adr);
strcatbuff(buff, fil);
hash_pos_return = coucal_read(cache->hashtable, buff, &hash_pos);
@@ -706,17 +707,25 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
value++;
ZIP_READFIELD_INT(line, value, "X-In-Cache", dataincache);
ZIP_READFIELD_INT(line, value, "X-Statuscode", r.statuscode);
ZIP_READFIELD_STRING(line, value, "X-StatusMessage", r.msg); // msg
ZIP_READFIELD_STRING(line, value, "X-StatusMessage", r.msg,
sizeof(r.msg));
ZIP_READFIELD_LLINT(line, value, "X-Size", r.size); // size
ZIP_READFIELD_STRING(line, value, "Content-Type", r.contenttype); // contenttype
ZIP_READFIELD_STRING(line, value, "X-Charset", r.charset); // contenttype
ZIP_READFIELD_STRING(line, value, "Last-Modified", r.lastmodified); // last-modified
ZIP_READFIELD_STRING(line, value, "Etag", r.etag); // Etag
ZIP_READFIELD_STRING(line, value, "Location", r.location); // 'location' pour moved
ZIP_READFIELD_STRING(line, value, "Content-Disposition", r.cdispo); // Content-disposition
ZIP_READFIELD_STRING(line, value, "Content-Type", r.contenttype,
sizeof(r.contenttype));
ZIP_READFIELD_STRING(line, value, "X-Charset", r.charset,
sizeof(r.charset));
ZIP_READFIELD_STRING(line, value, "Last-Modified", r.lastmodified,
sizeof(r.lastmodified));
ZIP_READFIELD_STRING(line, value, "Etag", r.etag, sizeof(r.etag));
// r.location is a char* pointing into a HTS_URLMAXSIZE*2 buffer
ZIP_READFIELD_STRING(line, value, "Location", r.location,
HTS_URLMAXSIZE * 2);
ZIP_READFIELD_STRING(line, value, "Content-Disposition", r.cdispo,
sizeof(r.cdispo));
//ZIP_READFIELD_STRING(line, value, "X-Addr", ..); // Original address
//ZIP_READFIELD_STRING(line, value, "X-Fil", ..); // Original URI filename
ZIP_READFIELD_STRING(line, value, "X-Save", previous_save_); // Original save filename
ZIP_READFIELD_STRING(line, value, "X-Save", previous_save_,
sizeof(previous_save_));
}
} while(offset < readSizeHeader && !lineEof);
//totalHeader = offset;
@@ -733,7 +742,7 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
}
}
if (return_save != NULL) {
strcpybuff(return_save, previous_save);
strlcpybuff(return_save, previous_save, HTS_URLMAXSIZE * 2);
}
/* Complete fields */
@@ -1025,7 +1034,7 @@ static htsblk cache_readex_old(httrackp * opt, cache_back * cache,
} else {
r.location = location_default;
}
strcpybuff(r.location, "");
r.location[0] = '\0';
#if HTS_FAST_CACHE
strcpybuff(buff, adr);
strcatbuff(buff, fil);
@@ -1111,7 +1120,7 @@ static htsblk cache_readex_old(httrackp * opt, cache_back * cache,
previous_save[0] = '\0';
cache_rstr(cache->olddat, previous_save); // save
if (return_save != NULL) {
strcpybuff(return_save, previous_save);
strlcpybuff(return_save, previous_save, HTS_URLMAXSIZE * 2);
}
}
if (cache->version >= 5) {
@@ -2088,7 +2097,7 @@ char *readfile_or(const char *fil, const char *defaultdata) {
char *adr = malloct(strlen(defaultdata) + 1);
if (adr) {
strcpybuff(adr, defaultdata);
strlcpybuff(adr, defaultdata, strlen(defaultdata) + 1);
return adr;
}
}

62
tests/02_update-cache.test Executable file
View File

@@ -0,0 +1,62 @@
#!/bin/bash
#
# Update path: re-mirroring a site reads the cache (cache_readex) to decide what
# is up to date -- a path the one-shot crawl tests never exercise. Offline
# (file://), so it always runs.
#
# 1. mirror, then re-mirror unchanged -> the cache-read pass must complete clean
# (guards against a crash/abort/error in cache_readex).
# 2. change a source file, re-mirror -> the update must pick up the new content
# (guards the update decision that reads the cached metadata).
set -eu
site=$(mktemp -d)
out=$(mktemp -d)
trap 'rm -rf "$site" "$out"' EXIT
cat >"$site/index.html" <<EOF
<a href="a.html">a</a> <a href="sub/b.html">b</a>
EOF
echo 'OLDCONTENT' >"$site/a.html"
mkdir -p "$site/sub"
echo '<p>bbb</p>' >"$site/sub/b.html"
url="file://$site/index.html"
# count Error: lines in the log (grep -c exits 1 on zero matches: guard it)
errors() { grep -ciE '^[0-9:]*[[:space:]]Error:' "$out/hts-log.txt" || true; }
# 1. fresh mirror writes the cache
httrack "$url" -O "$out" -q -%v0 -r3 >/dev/null 2>&1
test -e "$out/hts-cache/new.zip" || {
echo "no cache was written" >&2
exit 1
}
# 2. re-mirror unchanged: the update reads the cache and must complete cleanly
httrack "$url" -O "$out" -q -%v0 -r3 >/dev/null 2>&1
test "$(errors)" = 0 || {
echo "update (unchanged) reported errors" >&2
exit 1
}
for suffix in a.html sub/b.html; do
find "$out" -path "*/$suffix" | grep -q . || {
echo "missing $suffix after update" >&2
exit 1
}
done
# 3. change a source file: the update must pick up the new content
sleep 1
echo 'NEWCONTENT' >"$site/a.html"
httrack "$url" -O "$out" -q -%v0 -r3 >/dev/null 2>&1
test "$(errors)" = 0 || {
echo "update (changed) reported errors" >&2
exit 1
}
grep -q NEWCONTENT "$(find "$out" -path '*/a.html')" || {
echo "update did not pick up the changed source" >&2
exit 1
}

View File

@@ -22,6 +22,7 @@ TESTS = \
01_engine-simplify.test \
01_engine-strsafe.test \
02_manpage-regen.test \
02_update-cache.test \
10_crawl-simple.test \
11_crawl-cookies.test \
11_crawl-idna.test \