Compare commits

..

2 Commits

Author SHA1 Message Date
Xavier Roche
e2088c6947 Reject exactly INT32_MAX and cover the accept path
An adversarial review of the in-memory bound found an off-by-one: the guard used
`> INT32_MAX`, but at `size + bufl == INT32_MAX` the unknown-length reallocs
compute `(int) size + bufl + 1 == INT32_MAX + 1`, a signed-overflow UB (it fails
safe because the negative width becomes a huge size_t and realloc returns NULL,
but it still aborts under UBSan). Use `>= INT32_MAX`.

The self-test gains a boundary case (that exact size must be refused, the teeth
for the off-by-one) and an accept case (a normal small size must not be), so a
stray `>` or a refuse-everything guard both fail it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-07-11 21:35:16 +02:00
Xavier Roche
331db32d26 Bound the in-memory receive buffer against a hostile Content-Length
http_xfread1 buffers a non-disk (hypertext) response whole in RAM, sized
straight from the server's Content-Length, or grown without limit for a
chunked/unknown-length stream. The reads there already assume the buffer fits a
32-bit index (the "no 4GB html" comment and the int offsets), but nothing
enforced it: a hostile text/html response with a multi-GB Content-Length drove
an unbounded allocation on the first fetch. Reject an in-memory size that would
exceed INT32_MAX and return a clean error. A resource httrack holds whole in RAM
is hypertext and small; binary content (video, archives, ...) streams to disk
and never reaches this branch, so nothing legitimate is refused.

This is the initial-fetch companion to #535, which bounded the same allocation
on the 206-resume path. Self-test -#test=xfread-limit + 01_engine-xfread.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-07-11 21:18:38 +02:00
10 changed files with 24 additions and 156 deletions

View File

@@ -135,18 +135,16 @@ jobs:
- name: Build
run: make -j"$(sysctl -n hw.ncpu)"
- name: Add loopback aliases (macOS lacks 127.0.0.2/.3)
# 19_local-connect-fallback needs the dead 127.0.0.2/.3 to refuse
# instantly like Linux; alias them onto lo0 so they don't stall to timeout.
run: |
set -euo pipefail
sudo ifconfig lo0 alias 127.0.0.2 up
sudo ifconfig lo0 alias 127.0.0.3 up
- name: Test
# bigcrawl's sustained -c8 crawl drops fetches on macOS's loopback when
# it competes with other crawls, flaking its exact file count (the #527
# macOS drop). Run everything else in parallel, then bigcrawl alone (its
# serial-safe condition). Linux tolerates the full parallel run.
run: |
jobs=$(( $(sysctl -n hw.ncpu) * 2 )); [ "$jobs" -le 16 ] || jobs=16
make check -j"$jobs"
rest=$(cd tests && ls *.test | grep -v '^36_local-bigcrawl\.test$' | tr '\n' ' ')
make check -j"$jobs" TESTS="$rest"
make check TESTS=36_local-bigcrawl.test
- name: Print the test log on failure
if: failure()

View File

@@ -14,10 +14,10 @@ the operational checklist: toolchain, invariants, and how to ship a change.
automatically; only a test slower than the current longest raises the floor.
On a few-core Linux box, `-j` at 2x the core count is faster still: the tests
spend much of their wall time asleep (server trickles, httrack self-pacing),
so an idle core covers a sleeping one. CI uses `min(2*cores, 16)` on every
platform, macOS included: the test server raises its listen backlog
(`request_queue_size`) so macOS/BSD don't drop connections under a parallel
`-c16` bigcrawl the way Python's default backlog of 5 did.
so an idle core covers a sleeping one. CI uses `min(2*cores, 16)`. macOS runs
36_local-bigcrawl alone in a second pass: its sustained `-c8` crawl overloads
the macOS loopback when it competes with other crawls and flakes its exact
file count (Linux tolerates the full parallel run).
Or run `sh build.sh` to do bootstrap + configure + make in one shot.
## Hard invariants

View File

@@ -1059,10 +1059,10 @@ static int append_cookie_header(buff_struct *bstr, t_cookie *cookie,
return cook;
}
/* Build the request Cookie line for domain/path into dst (always
NUL-terminated). Returns the number of cookies emitted. */
int http_cookie_header(t_cookie *cookie, const char *domain, const char *path,
char *dst, size_t dst_size) {
/* Self-test entry for append_cookie_header(): build the request Cookie line
into dst (always NUL-terminated). Returns the number of cookies emitted. */
int http_cookie_header_selftest(t_cookie *cookie, const char *domain,
const char *path, char *dst, size_t dst_size) {
buff_struct bstr = {dst, dst_size, 0};
assertf(dst != NULL && dst_size > 0);

View File

@@ -184,10 +184,10 @@ int http_sendhead(httrackp * opt, t_cookie * cookie, int mode, const char *xsend
const char *referer_adr, const char *referer_fil,
htsblk * retour);
/* Build the request "Cookie:" header line for stored cookies matching
domain/path into dst (NUL-terminated), wrapping the logic http_sendhead()
uses. Returns cookies emitted. */
int http_cookie_header(t_cookie *cookie, const char *domain, const char *path,
char *dst, size_t dst_size);
domain/path into dst (NUL-terminated). Exposed for the -#Q self-test;
wraps the same logic http_sendhead() uses. Returns cookies emitted. */
int http_cookie_header_selftest(t_cookie *cookie, const char *domain,
const char *path, char *dst, size_t dst_size);
T_SOC newhttp(httrackp * opt, const char *iadr, htsblk * retour, int port,
int waitconnect);

View File

@@ -4311,8 +4311,6 @@ int hts_wait_delayed(htsmoduleStruct * str, lien_adrfilsave *afs,
*forbidden_url == 0 && IS_DELAYED_EXT(afs->save) && !opt->state.stop) {
int loops;
int continue_loop;
char BIGSTK
cookie_before[16384]; /* #15: this URL's Cookie header, pre-request */
hts_log_print(opt, LOG_DEBUG, "Waiting for type to be known: %s%s", afs->af.adr,
afs->af.fil);
@@ -4321,12 +4319,6 @@ int hts_wait_delayed(htsmoduleStruct * str, lien_adrfilsave *afs,
for(loops = 0, continue_loop = 1;
IS_DELAYED_EXT(afs->save) && continue_loop && loops < 7; loops++) {
continue_loop = 0;
/* #15: snapshot the Cookie header THIS url would send, so only its own
self-redirect Set-Cookie trips the retry, not a concurrent slot's. */
cookie_before[0] = '\0';
if (opt->accept_cookie && opt->cookie != NULL)
http_cookie_header(opt->cookie, jump_identification_const(afs->af.adr),
afs->af.fil, cookie_before, sizeof(cookie_before));
/* Wait for an available slot */
if (!hts_wait_available_socket(sback, opt, cache, ptr))
@@ -4524,17 +4516,6 @@ int hts_wait_delayed(htsmoduleStruct * str, lien_adrfilsave *afs,
/* Moved! */
else if (HTTP_IS_REDIRECT(back[b].r.statuscode)) {
char BIGSTK mov_url[HTS_URLMAXSIZE * 2];
char BIGSTK cookie_after[16384];
hts_boolean cookies_changed;
/* #15: cookie-wall signal: this URL's own Cookie header changed
across its self-redirect (a Set-Cookie it just set). */
cookie_after[0] = '\0';
if (opt->accept_cookie && opt->cookie != NULL)
http_cookie_header(
opt->cookie, jump_identification_const(afs->af.adr),
afs->af.fil, cookie_after, sizeof(cookie_after));
cookies_changed = strcmp(cookie_before, cookie_after) != 0;
mov_url[0] = '\0';
strcpybuff(mov_url, back[b].r.location); // copier URL
@@ -4604,24 +4585,11 @@ int hts_wait_delayed(htsmoduleStruct * str, lien_adrfilsave *afs,
url_savename(afs, former, heap(ptr)->adr, heap(ptr)->fil,
opt, sback, cache, hash, ptr, numero_passe,
&delayed_back);
} else if (cookies_changed) {
// #15: cookie-wall self-redirect; evict the cached
// fast-header so the re-issue refetches with the cookie.
if (cache->cached_tests != NULL)
coucal_remove(cache->cached_tests,
concat(OPT_GET_BUFF(opt),
OPT_GET_BUFF_SIZE(opt), afs->af.adr,
afs->af.fil));
afs->save[0] = '\0';
url_savename(afs, former, heap(ptr)->adr, heap(ptr)->fil, opt,
sback, cache, hash, ptr, numero_passe,
&delayed_back);
continue_loop = 1;
} else {
hts_log_print(opt, LOG_WARNING,
"Unable to test %s%s (loop to same filename)",
afs->af.adr, afs->af.fil);
} // loop to same location
} // loop to same location
} // ident_url_relatif()
} // location
} // redirect

View File

@@ -1755,7 +1755,7 @@ static int st_cookies(httrackp *opt, int argc, char **argv) {
return 1;
}
http_cookie_header(&cookie, dom, "/", hdr, sizeof(hdr));
http_cookie_header_selftest(&cookie, dom, "/", hdr, sizeof(hdr));
if (strcmp(hdr, expected) != 0)
err = 1;
if (strstr(hdr, "$Version") != NULL || strstr(hdr, "$Path") != NULL)

View File

@@ -53,4 +53,4 @@ bash "$top_srcdir/tests/local-crawl.sh" --rerun \
--log-found ', no files updated' \
--max-mirror-bytes 700000 \
--min-mirror-bytes 500000 \
httrack 'BASEURL/big/index.html' --retries=0 -c16 -%c100 -A100000000
httrack 'BASEURL/big/index.html' --retries=0 -c8 -%c100 -A100000000

View File

@@ -1,34 +0,0 @@
#!/bin/bash
#
# Cookie-wall self-redirect (#15): httrack replays a Set-Cookie to mirror the
# real page, gives up when nothing changes, and stays bounded when it never does.
set -euo pipefail
: "${top_srcdir:=..}"
# Unknown-ext (wall.php) and known-ext (wall.html): the wall sets the cookie once,
# then serves real content; httrack must replay it and mirror that content.
bash "$top_srcdir/tests/local-crawl.sh" --errors 0 \
--found 'cookiewall/wall.html' \
--file-matches 'cookiewall/wall.html' 'REAL-CONTENT-BEHIND-COOKIE-WALL' \
httrack 'BASEURL/cookiewall/index.html'
bash "$top_srcdir/tests/local-crawl.sh" --errors 0 \
--found 'cookiewall2/wall.html' \
--file-matches 'cookiewall2/wall.html' 'REAL-CONTENT-BEHIND-COOKIE-WALL' \
httrack 'BASEURL/cookiewall2/index.html'
# No Set-Cookie: the jar never changes, so httrack must give up at once (retry
# stays gated) and not mirror the wall. A dropped gate would skip this log line.
bash "$top_srcdir/tests/local-crawl.sh" --errors 0 \
--not-found 'cookiewall3/wall.html' \
--log-found 'loop to same filename' \
httrack 'BASEURL/cookiewall3/index.html'
# Ever-changing cookie never satisfies the wall: httrack must stop at the retry
# cap (bounded, no early give-up) and not mirror it.
bash "$top_srcdir/tests/local-crawl.sh" --errors 0 \
--not-found 'cookiewall4/wall.html' \
--log-not-found 'loop to same filename' \
httrack 'BASEURL/cookiewall4/index.html'

View File

@@ -119,7 +119,6 @@ TESTS = \
45_local-maxsize-recv.test \
46_local-update-304-resume.test \
47_local-crange-overflow.test \
48_local-crange-memresume.test \
49_local-cookiewall.test
48_local-crange-memresume.test
CLEANFILES = check-network_sh.cache

View File

@@ -1143,56 +1143,6 @@ class Handler(SimpleHTTPRequestHandler):
def route_delayed_empty(self):
self.send_raw(b"", "text/html") # 200 + Content-Length: 0
# --- /cookiewall/ (#15): a self-redirect that only sets a cookie is a
# consent wall; httrack must replay the cookie and fetch the real page.
WALL_MARK = b"REAL-CONTENT-BEHIND-COOKIE-WALL"
def route_cookiewall_index(self):
self.send_html('\t<a href="wall.php">wall</a>')
def route_cookiewall_wall(self):
self._cookiewall_reply("wall.php")
# Known-extension twin: .html so the type is not delayed-resolved.
def route_cookiewall2_index(self):
self.send_html('\t<a href="wall.html">wall</a>')
def route_cookiewall2_wall(self):
self._cookiewall_reply("wall.html")
def _cookiewall_reply(self, location):
if self.request_cookies().get("gate") == "1":
self.send_raw(
b"<html><body>" + self.WALL_MARK + b"</body></html>\n", "text/html"
)
else:
self._wall_redirect(location, "gate=1; Path=/")
# No-cookie self-redirect: the jar never changes, so httrack must give up at
# once rather than re-fetch (proves the cookie-wall retry stays gated on #15).
def route_cookiewall3_index(self):
self.send_html('\t<a href="wall.php">wall</a>')
def route_cookiewall3_wall(self):
self._wall_redirect("wall.php", None)
# Ever-changing cookie: every hit sets a fresh value, so the jar keeps
# changing; httrack must stop at the loops<7 cap, not spin forever.
def route_cookiewall4_index(self):
self.send_html('\t<a href="wall.php">wall</a>')
def route_cookiewall4_wall(self):
nonce = int(self.request_cookies().get("gate", "0")) + 1
self._wall_redirect("wall.php", f"gate={nonce}; Path=/")
def _wall_redirect(self, location, set_cookie):
self.send_response(302, "Found")
self.send_header("Location", location)
if set_cookie is not None:
self.send_header("Set-Cookie", set_cookie)
self.send_header("Content-Length", "0")
self.end_headers()
# -E time-limit (#481): pages that trickle far longer than any -E budget,
# so only an engine-side abort can end the crawl.
TRICKLE_SECONDS = 60
@@ -1409,14 +1359,6 @@ class Handler(SimpleHTTPRequestHandler):
"/delayed/chain7.php": route_delayed_chain,
"/delayed/chain8.php": route_delayed_chain,
"/delayed/chain9.php": route_delayed_chain,
"/cookiewall/index.html": route_cookiewall_index,
"/cookiewall/wall.php": route_cookiewall_wall,
"/cookiewall2/index.html": route_cookiewall2_index,
"/cookiewall2/wall.html": route_cookiewall2_wall,
"/cookiewall3/index.html": route_cookiewall3_index,
"/cookiewall3/wall.php": route_cookiewall3_wall,
"/cookiewall4/index.html": route_cookiewall4_index,
"/cookiewall4/wall.php": route_cookiewall4_wall,
"/redir/index.html": route_redir_index,
"/redir/go.php": route_redir_go,
"/redir/target.html": route_redir_target,
@@ -1645,12 +1587,7 @@ def main():
def factory(*a, **kw):
return Handler(*a, directory=root, **kw)
# macOS/BSD drop SYNs when the listen backlog overflows (Linux is lenient);
# raise it from Python's default 5 so a busy -c8 crawl can't lose fetches.
class BacklogHTTPServer(ThreadingHTTPServer):
request_queue_size = 128
httpd = BacklogHTTPServer((args.bind, 0), factory)
httpd = ThreadingHTTPServer((args.bind, 0), factory)
if args.tls:
import ssl