mirror of
https://github.com/xroche/httrack.git
synced 2026-08-06 15:56:26 +03:00
5a38473343
* Bound the ProxyTrack DAV item buffer against an amplified PROPFIND path proxytrack_add_DAV_Item() reserved a fixed 1024 bytes and then sprintf'd into it unbounded. The request path lands in the response twice, once as the href and once as the displayname, and escapexml() turns each '&' into '&', so an unauthenticated PROPFIND of roughly 900 ampersands writes about 9000 bytes off the end of the heap block. No cache entry and no Depth: 1 are needed. Replace the hand-sized reserve with StringSprintf(), which measures the formatted output and grows the String to fit, and convert the sibling sprintf sites in the same file so no unbounded write into a String is left to re-audit. Sizing beats clipping here: the String already owns a growable buffer, so nothing has to be dropped. Closes #836 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Bound StringSprintf's pre-C99 retry, and trim the review findings A genuine vsnprintf conversion error returns -1 just as pre-C99 msvcrt does for a short buffer, so the doubling search had no way to tell them apart and grew until realloc aborted. Unreachable from these format strings, which use only %s and %d, but the helper lives in a shared header and will get more callers. Cap the search and empty the String past it. Also: the count assertion piped into wc under pipefail, so a zero count killed the test through set -e before its diagnostic could print. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Lift the NO_WEBDAV conditional out of a macro argument list A preprocessor directive inside a macro invocation's arguments is undefined: it was fine while this was a plain sprintf() call, and MSVC rejected it as soon as it became StringSprintf(). GCC accepts it, so only the Windows leg caught it. Compute the DAV header fragment first and pass it as an argument. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Cover StringSprintf's exact-fill case and the WebDAV enumeration branch StringSprintf_ writes the terminator at buffer[ret], so widening its `ret < capacity` guard by one byte is a heap overflow that only fires when the formatted output exactly fills the capacity. No crawl test lands on a capacity boundary, so the mutant survived the suite. The new `strsprintf` self-test sweeps lengths around 256, 512, 1024 and 2048 with the String's capacity pinned to each, plus a growing and shrinking sweep on one reused String, and checks the length, the bytes and the terminator every time. Test 147 only ever sent Depth: 0, leaving the enumeration branch the same PR rewrote with no coverage at all. Its fixture gains a child directory, and a Depth: 1 listing pins the item URLs, including the trailing '/' that StringPopRight takes back off a directory name. Signed-off-by: Xavier Roche <xroche@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Make the String failure paths safe without assert StringSprintf empties the String when it gives up, but that contract was only visible in the implementation, and the WebDAV enumeration in proxytrack pops the trailing '/' straight after it. State it at the declaration, no-op StringPopRight on an empty String, and skip an enumerated item the formatter could not name. StringRoomTotal reported a failed realloc through STRING_ASSERT alone. The MSVC Release configuration defines NDEBUG, so that check is already gone from the shipped Windows builds, leaving a NULL buffer under a capacity bumped before the allocation was known to succeed. Assign both only on success, and terminate through StringOom_. Closes #915 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Renumber the String OOM test to 152 151 is taken by the unmerged tests/151_bash-shell-validate.test (PR #920). The filenames differ, so git would have carried both onto master rather than conflicting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Declare the new WebDAV test in the Windows skip set It skips on Windows for the same reason as its two neighbours, MSYS cannot reap a background listener (#595), and the ratchet fails a skip it was not told about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199wAkSVZNBNp51mpRkxMvv Signed-off-by: Xavier Roche <roche@httrack.com> * Keep the out-of-memory action overridable STRING_REALLOC and STRING_FREE are #ifndef hooks, and STRING_ASSERT was one too; replacing it with a hard-wired call took a hook away from downstreams of this installed header. Route the failure through STRING_OOM instead, with the print-and-abort default unchanged. Also flush stderr before aborting: the Windows CRT buffers a redirected stderr and abort() flushes nothing, which would drop the message the test matches on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Inject the allocation failure instead of asking for a huge one The engine self-test pinned a String's capacity so the next doubling asked for SIZE_MAX/2, on the assumption that no allocator would serve it. Six CI legs disagreed: i386 has a 3G user space, and the 64-bit runners handed the request out too, so the test reported "NOT aborted" everywhere but here. Green that depends on how much memory the machine feels like giving is not a test. Drive the path from a standalone helper instead, which defines STRING_REALLOC to a stub returning NULL before including htsstrings.h. Four cases: growth with the stub allocating for real, the failure reaching the handler with the size it asked for, a live buffer surviving a failed realloc, and the shipped handler printing and aborting. Only the automake build produces the helper, so the test declares its Windows skip. The self-test had no portable way to force the failure, so it goes rather than staying as a handler nobody can rely on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> * Assert the bytes and the requested size, not just the bookkeeping An under-allocation survived the helper: shortening the realloc by one byte while still recording the full capacity left all four cases green, because only the growth case allocated anything and it checked the capacity number rather than the memory behind it. Fill the announced capacity to its last byte and read it back, which the sanitizer legs turn into a hard failure. The failure cases pinned the initial capacity by asserting 16, so bumping that policy would have failed a correct tree. Compare the size handed to the handler against the size the stub was actually asked for instead, which also catches the under-allocation on legs with no sanitizer. Drive StringSprintf_ and StringBuffN_ too, the other two places the header expands STRING_OOM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com> --------- Signed-off-by: Xavier Roche <roche@httrack.com> Signed-off-by: Xavier Roche <xroche@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
48 lines
1.6 KiB
Bash
Executable File
48 lines
1.6 KiB
Bash
Executable File
#!/bin/bash
|
|
#
|
|
# A String that cannot grow must stop the process loudly rather than carry on
|
|
# with a NULL buffer and a capacity already bumped. assert() is compiled out of
|
|
# the MSVC Release build, so the check cannot lean on it (#915). The failure is
|
|
# injected through a realloc stub: asking a real allocator for a size it should
|
|
# refuse is a guess about the machine, not a test.
|
|
|
|
set -euo pipefail
|
|
|
|
ulimit -c 0 # a deliberate abort must not litter the box with cores
|
|
|
|
fail() {
|
|
echo "FAIL: $1" >&2
|
|
exit 1
|
|
}
|
|
|
|
# Unset means this is not an automake run (the Windows job runs the scripts
|
|
# directly), so the helper does not exist. Set but missing is a build problem,
|
|
# not a skip, or the assertions below would pass vacuously.
|
|
if [ -z "${STRINGOOM_BIN:-}" ]; then
|
|
echo "STRINGOOM_BIN unset, no automake environment, skipping" >&2
|
|
exit 77
|
|
fi
|
|
[ -r "$STRINGOOM_BIN" ] || fail "$STRINGOOM_BIN was not built"
|
|
|
|
# Control first: with the stub allocating for real, growth must not reach the
|
|
# failure path at all, and the capacity it announces must be writable to the
|
|
# last byte. Then the three failure shapes, and the growers inside the header.
|
|
for mode in grow hook keep sprintf buffn; do
|
|
out=$("$STRINGOOM_BIN" "$mode") || fail "$mode case exited non-zero: $out"
|
|
test "$out" = "$mode: OK" || fail "expected '$mode: OK', got: $out"
|
|
done
|
|
|
|
# The shipped handler prints and aborts, so don't let set -e trip on it.
|
|
err=$("$STRINGOOM_BIN" abort 2>&1) || true
|
|
case "$err" in
|
|
*"NOT aborted"*)
|
|
fail "the failed allocation was not caught: $err"
|
|
;;
|
|
*"String: out of memory"*) ;;
|
|
*)
|
|
fail "expected the String out-of-memory abort, got: $err"
|
|
;;
|
|
esac
|
|
|
|
exit 0
|