Files
httrack/tests/stringoom.c
Xavier Roche 5a38473343 A failed String allocation is only caught by assert, and the empty-on-failure contract is unstated (#921)
* 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>
2026-08-02 11:29:24 +00:00

236 lines
7.1 KiB
C

/* ------------------------------------------------------------ */
/*
HTTrack Website Copier, Offline Browser for Windows and Unix
Copyright (C) 1998 Xavier Roche and other contributors
SPDX-License-Identifier: GPL-3.0-or-later
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
Please visit our Website: http://www.httrack.com
*/
/* Drives the String allocation-failure path through a realloc stub: asking a
real allocator for a size it should refuse is a guess about the machine, not
a test (#915). One case per run, named by argv[1]. */
#include <setjmp.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
static int alloc_fails = 0;
static int oom_jumps = 0;
static int oom_calls = 0;
static size_t oom_size = 0;
static size_t last_request = 0;
static jmp_buf oom_jump;
static char *test_realloc(char *buff, size_t size) {
last_request = size;
if (alloc_fails) {
return NULL;
}
return (char *) realloc(buff, size);
}
/* Declared before the include: StringBuffN_ and StringSprintf_ expand
STRING_OOM inside the header itself. */
static void test_oom(size_t size);
#define STRING_REALLOC(BUFF, SIZE) test_realloc(BUFF, SIZE)
#define STRING_FREE(BUFF) free(BUFF)
#define STRING_OOM(SIZE) test_oom(SIZE)
#include "htsstrings.h"
/* Either returns to the caller, leaving the String observable, or runs the
shipped handler. */
static void test_oom(size_t size) {
oom_calls++;
oom_size = size;
if (oom_jumps) {
longjmp(oom_jump, 1);
}
StringOom_(size);
}
/* File scope, so longjmp cannot leave them indeterminate. */
static String room = STRING_EMPTY;
static const char *kept = NULL;
static size_t kept_capacity = 0;
/* The handler must run once, and be told the size that was actually asked of
the allocator: an under-allocation reports a size the String does not have.
Compared against the stub's own record rather than a literal, so the initial
capacity stays a policy the test does not pin. */
static int failure_reported(const char *name) {
if (oom_calls != 1) {
printf("%s: FAIL (handler ran %d times)\n", name, oom_calls);
return 0;
}
if (oom_size != last_request) {
printf("%s: FAIL (reported %u bytes, allocator was asked for %u)\n", name,
(unsigned) oom_size, (unsigned) last_request);
return 0;
}
return 1;
}
/* Control: with the stub allocating for real, nothing must reach the handler.
Without this, a stub stuck in failing mode would "prove" every case. */
static int grow_case(void) {
size_t cap, i;
StringRoomTotal(room, 100);
if (oom_calls != 0) {
printf("grow: FAIL (handler ran %d times)\n", oom_calls);
return 1;
}
cap = StringCapacity(room);
if (StringBuff(room) == NULL || cap < 100) {
printf("grow: FAIL (capacity %u)\n", (unsigned) cap);
return 1;
}
/* A capacity never written to is a number, not a buffer: fill it to the
last byte, so an allocation short of the announced capacity is a heap
overflow the sanitizer legs catch. */
for (i = 0; i + 1 < cap; i++) {
StringBuffRW(room)[i] = (char) ('a' + (i % 26));
}
StringBuffRW(room)[cap - 1] = '\0';
for (i = 0; i + 1 < cap; i++) {
if (StringBuff(room)[i] != (char) ('a' + (i % 26))) {
printf("grow: FAIL (byte %u of %u read back as 0x%02x)\n", (unsigned) i,
(unsigned) cap, (unsigned char) StringBuff(room)[i]);
return 1;
}
}
if (strlen(StringBuff(room)) != cap - 1) {
printf("grow: FAIL (%u bytes readable, capacity %u)\n",
(unsigned) strlen(StringBuff(room)), (unsigned) cap);
return 1;
}
StringFree(room);
printf("grow: OK\n");
return 0;
}
static int hook_case(void) {
alloc_fails = oom_jumps = 1;
if (setjmp(oom_jump) == 0) {
StringRoomTotal(room, 100);
printf("hook: FAIL (grew through a failing allocator)\n");
return 1;
}
if (!failure_reported("hook")) {
return 1;
}
if (StringBuff(room) != NULL || StringCapacity(room) != 0 ||
StringLength(room) != 0) {
printf("hook: FAIL (capacity %u, buffer %s)\n",
(unsigned) StringCapacity(room),
StringBuff(room) == NULL ? "null" : "set");
return 1;
}
printf("hook: OK\n");
return 0;
}
/* The one the old code got wrong: a failed realloc must not overwrite the live
buffer with NULL nor bump the capacity past what was allocated. */
static int keep_case(void) {
StringCopy(room, "abc");
kept = StringBuff(room);
kept_capacity = StringCapacity(room);
alloc_fails = oom_jumps = 1;
if (setjmp(oom_jump) == 0) {
StringRoomTotal(room, 1000);
printf("keep: FAIL (grew through a failing allocator)\n");
return 1;
}
if (!failure_reported("keep")) {
return 1;
}
if (StringBuff(room) != kept || StringCapacity(room) != kept_capacity ||
StringLength(room) != 3 || strcmp(StringBuff(room), "abc") != 0) {
printf("keep: FAIL (buffer %s, capacity %u was %u)\n",
StringBuff(room) == kept ? "kept" : "moved",
(unsigned) StringCapacity(room), (unsigned) kept_capacity);
return 1;
}
alloc_fails = 0;
StringFree(room);
printf("keep: OK\n");
return 0;
}
/* The header's own growers expand STRING_OOM as well, so drive each of them
into the same failure rather than only the macro they call. */
static int sprintf_case(void) {
alloc_fails = oom_jumps = 1;
if (setjmp(oom_jump) == 0) {
StringSprintf(room, "%s", "x");
printf("sprintf: FAIL (formatted through a failing allocator)\n");
return 1;
}
if (!failure_reported("sprintf") || StringBuff(room) != NULL) {
return 1;
}
printf("sprintf: OK\n");
return 0;
}
static int buffn_case(void) {
alloc_fails = oom_jumps = 1;
if (setjmp(oom_jump) == 0) {
(void) StringBuffN(room, 10);
printf("buffn: FAIL (reserved through a failing allocator)\n");
return 1;
}
if (!failure_reported("buffn") || StringBuff(room) != NULL) {
return 1;
}
printf("buffn: OK\n");
return 0;
}
/* Runs the shipped handler, which must print and abort. */
static int abort_case(void) {
alloc_fails = 1;
StringRoomTotal(room, 100);
printf("abort: NOT aborted\n");
return 1;
}
int main(int argc, char **argv) {
const char *const mode = argc > 1 ? argv[1] : "";
if (strcmp(mode, "grow") == 0) {
return grow_case();
} else if (strcmp(mode, "hook") == 0) {
return hook_case();
} else if (strcmp(mode, "keep") == 0) {
return keep_case();
} else if (strcmp(mode, "sprintf") == 0) {
return sprintf_case();
} else if (strcmp(mode, "buffn") == 0) {
return buffn_case();
} else if (strcmp(mode, "abort") == 0) {
return abort_case();
}
fprintf(stderr, "usage: %s grow|hook|keep|sprintf|buffn|abort\n", argv[0]);
return 2;
}