diff options
| author | Jeff King <peff@peff.net> | 2025-11-18 04:12:28 -0500 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2025-11-18 09:36:12 -0800 |
| commit | 5a993593b24df699f60841296795f9a6ca60d399 (patch) | |
| tree | 071af353f88204b9b150106d6a638cf78b8e6ac9 | |
| parent | f05df7ffca492b37d604ad6beed788055eb56ebd (diff) | |
| download | git-5a993593b24df699f60841296795f9a6ca60d399.tar.gz | |
fsck: avoid parse_timestamp() on buffer that isn't NUL-terminated
In fsck_ident(), we parse the timestamp with parse_timestamp(), which is
really an alias for strtoumax(). But since our buffer may not be
NUL-terminated, this can trigger a complaint from ASan's
strict_string_checks mode. This is a false positive, since we know that
the buffer contains a trailing newline (which we checked earlier in the
function), and that strtoumax() would stop there.
But it is worth working around ASan's complaint. One is because that
will let us turn on strict_string_checks by default, which has helped
catch other real problems. And two is that the safety of the current
code is very hard to reason about (it subtly depends on distant code
which could change).
One option here is to just parse the number left-to-right ourselves. But
we care about the size of a timestamp_t and detecting overflow, since
that's part of the point of these checks. And doing that correctly is
tricky. So we'll instead just pull the digits into a separate,
NUL-terminated buffer, and use that to call parse_timestamp().
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
| -rw-r--r-- | fsck.c | 23 |
1 files changed, 19 insertions, 4 deletions
@@ -859,13 +859,28 @@ static int verify_headers(const void *data, unsigned long size, FSCK_MSG_UNTERMINATED_HEADER, "unterminated header"); } +static timestamp_t parse_timestamp_from_buf(const char **start, const char *end) +{ + const char *p = *start; + char buf[24]; /* big enough for 2^64 */ + size_t i = 0; + + while (p < end && isdigit(*p)) { + if (i >= ARRAY_SIZE(buf) - 1) + return TIME_MAX; + buf[i++] = *p++; + } + buf[i] = '\0'; + *start = p; + return parse_timestamp(buf, NULL, 10); +} + static int fsck_ident(const char **ident, const char *ident_end, const struct object_id *oid, enum object_type type, struct fsck_options *options) { const char *p = *ident; const char *nl; - char *end; nl = memchr(p, '\n', ident_end - p); if (!nl) @@ -917,11 +932,11 @@ static int fsck_ident(const char **ident, const char *ident_end, "invalid author/committer line - bad date"); if (*p == '0' && p[1] != ' ') return report(options, oid, type, FSCK_MSG_ZERO_PADDED_DATE, "invalid author/committer line - zero-padded date"); - if (date_overflows(parse_timestamp(p, &end, 10))) + if (date_overflows(parse_timestamp_from_buf(&p, ident_end))) return report(options, oid, type, FSCK_MSG_BAD_DATE_OVERFLOW, "invalid author/committer line - date causes integer overflow"); - if (*end != ' ') + if (*p != ' ') return report(options, oid, type, FSCK_MSG_BAD_DATE, "invalid author/committer line - bad date"); - p = end + 1; + p++; if ((*p != '+' && *p != '-') || !isdigit(p[1]) || !isdigit(p[2]) || |
