Compare commits

...

4 Commits

Author SHA1 Message Date
Xavier Roche
3c065e401b Merge remote-tracking branch 'origin/master' into fix/htscore-745 2026-07-27 09:08:33 +02:00
Xavier Roche
7818cbbfbb tests: drop the max-length rename case, macOS PATH_MAX is 1024
The path the guard admits (HTS_URLMAXSIZE) plus ".txt" is longer than macOS
accepts, so fopen() failed there. The case could not tell a fixed build from an
unfixed one anyway; what is left covers the guard and the rename on both entry
points.

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-07-27 09:03:57 +02:00
Xavier Roche
f58d81e4f6 review: tighten the structcheck self-test and its comments
The path builder could end a path with a bare separator when the base
directory's length hit the wrong residue, so the test aborted on a long
$TMPDIR. The refusal case also asserted on a component structcheck never
creates; assert on the outermost one instead.

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-07-27 08:53:41 +02:00
Xavier Roche
e9c3eb2e28 structcheck() builds its rename target with an unbounded sprintf
Both structcheck() and structcheck_utf8() move a regular file sitting where a
directory belongs, and build the "<name>.txt" target with a raw sprintf into a
2048-byte buffer. It stays in bounds only because of a strlen(path) >
HTS_URLMAXSIZE guard dozens of lines above, which nothing at the write site
mentions. Route both through sprintfbuff() and fail with ENAMETOOLONG, so the
bound is local.

Armed the probe: with that distant guard patched out, a 2045-byte path makes
the old sprintf write 2049 bytes into tmpbuf[2048] under ASan; the same build
with sprintfbuff() returns -1 and reports nothing.

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-07-27 08:44:32 +02:00
4 changed files with 101 additions and 2 deletions

View File

@@ -2693,7 +2693,11 @@ HTSEXT_API int structcheck(const char *path) {
if (!S_ISDIR(st.st_mode)) {
#if HTS_REMOVE_ANNOYING_INDEX
if (S_ISREG(st.st_mode)) { /* Regular file in place ; move it and create directory */
sprintf(tmpbuf, "%s.txt", file);
/* bounded here, not by the path-length guard far above */
if (!sprintfbuff(tmpbuf, "%s.txt", file)) {
errno = ENAMETOOLONG;
return -1;
}
if (rename(file, tmpbuf) != 0) { /* Can't rename regular file */
return -1;
}
@@ -2801,7 +2805,11 @@ HTSEXT_API int structcheck_utf8(const char *path) {
if (!S_ISDIR(st.st_mode)) {
#if HTS_REMOVE_ANNOYING_INDEX
if (S_ISREG(st.st_mode)) { /* Regular file in place ; move it and create directory */
sprintf(tmpbuf, "%s.txt", file);
/* bounded here, not by the path-length guard far above */
if (!sprintfbuff(tmpbuf, "%s.txt", file)) {
errno = ENAMETOOLONG;
return -1;
}
if (RENAME(file, tmpbuf) != 0) { /* Can't rename regular file */
return -1;
}

View File

@@ -3262,6 +3262,81 @@ static int st_topindex(httrackp *opt, int argc, char **argv) {
return 0;
}
/* Build a path of exactly len chars under base; returns that length. */
static size_t st_structcheck_longpath(char *dst, size_t dstsize,
const char *base, size_t len) {
size_t n = strlen(base);
assertf(len < dstsize && n + 2 <= len);
memmove(dst, base, n);
while (n < len) {
size_t seg = len - n - 1;
if (seg > 200) /* stay under the usual 255-byte component limit */
seg = len - n == 202 ? 199 : 200; /* never leave a bare separator */
dst[n++] = '/';
memset(dst + n, 'x', seg);
n += seg;
}
dst[n] = '\0';
return n;
}
/* The path guard, and the <name>.txt rename structcheck() performs when a
regular file sits where a directory has to go (#745). */
static int st_structcheck(httrackp *opt, int argc, char **argv) {
char BIGSTK path[HTS_URLMAXSIZE * 2];
char BIGSTK target[HTS_URLMAXSIZE * 2];
FILE *fp;
(void) opt;
if (argc < 1) {
fprintf(stderr, "usage: -#test=structcheck <writable directory>\n");
return 1;
}
/* over the guard: refused before a single directory is created */
st_structcheck_longpath(path, sizeof(path), argv[0], HTS_URLMAXSIZE + 1);
errno = 0;
assertf(structcheck(path) == -1);
assertf(errno == EINVAL);
errno = 0;
assertf(structcheck_utf8(path) == -1);
assertf(errno == EINVAL);
{
char *const sep = strchr(path + strlen(argv[0]) + 1, '/');
assertf(sep != NULL);
sep[1] = '\0'; /* the outermost component it would have created */
assertf(!dir_exists(path));
}
/* a regular file where a directory belongs is renamed to <name>.txt */
snprintf(path, sizeof(path), "%s/sc", argv[0]);
fp = fopen(path, "wb");
assertf(fp != NULL);
fclose(fp);
snprintf(path, sizeof(path), "%s/sc/sub/", argv[0]);
assertf(structcheck(path) == 0);
assertf(dir_exists(path));
snprintf(target, sizeof(target), "%s/sc.txt", argv[0]);
assertf(fexist(target));
/* the utf-8 entry point carries the same rename */
snprintf(path, sizeof(path), "%s/u8", argv[0]);
fp = FOPEN(path, "wb");
assertf(fp != NULL);
fclose(fp);
snprintf(path, sizeof(path), "%s/u8/sub/", argv[0]);
assertf(structcheck_utf8(path) == 0);
assertf(dir_exists(path));
snprintf(target, sizeof(target), "%s/u8.txt", argv[0]);
assertf(fexist_utf8(target));
printf("structcheck self-test OK\n");
return 0;
}
/* Each inplace_escape_*() must equal escape_*() on a copy. */
static int st_inplace_escape(httrackp *opt, int argc, char **argv) {
/* >255 bytes forces the helper's malloct path, not the stack buffer */
@@ -6421,6 +6496,9 @@ static const struct selftest_entry {
{"useragent", "", "default User-Agent self-test", st_useragent},
{"makeindex", "[dir]", "hts_finish_makeindex footer/refresh self-test",
st_makeindex},
{"structcheck", "<dir>",
"structcheck path guard and the <name>.txt rename it performs",
st_structcheck},
{"topindex", "[dir]",
"hts_buildtopindex charset handling of a non-ASCII project dir",
st_topindex},

View File

@@ -0,0 +1,12 @@
#!/bin/bash
#
# structcheck(): the path-length guard, and the <name>.txt rename that once
# built its target with an unbounded sprintf (#745).
set -euo pipefail
dir=$(mktemp -d)
trap 'rm -rf "$dir"' EXIT
httrack -O /dev/null -#test=structcheck "$dir" |
grep -q "structcheck self-test OK"

View File

@@ -81,6 +81,7 @@ TESTS = \
01_engine-status.test \
01_engine-stripquery.test \
01_engine-strsafe.test \
01_engine-structcheck.test \
01_engine-syscharset.test \
01_engine-threadwait.test \
01_engine-topindex.test \