Skip to content

Commit 5a99359

Browse files
peffgitster
authored andcommitted
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>
1 parent f05df7f commit 5a99359

File tree

1 file changed

+19
-4
lines changed

1 file changed

+19
-4
lines changed

fsck.c

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -859,13 +859,28 @@ static int verify_headers(const void *data, unsigned long size,
859859
FSCK_MSG_UNTERMINATED_HEADER, "unterminated header");
860860
}
861861

862+
static timestamp_t parse_timestamp_from_buf(const char **start, const char *end)
863+
{
864+
const char *p = *start;
865+
char buf[24]; /* big enough for 2^64 */
866+
size_t i = 0;
867+
868+
while (p < end && isdigit(*p)) {
869+
if (i >= ARRAY_SIZE(buf) - 1)
870+
return TIME_MAX;
871+
buf[i++] = *p++;
872+
}
873+
buf[i] = '\0';
874+
*start = p;
875+
return parse_timestamp(buf, NULL, 10);
876+
}
877+
862878
static int fsck_ident(const char **ident, const char *ident_end,
863879
const struct object_id *oid, enum object_type type,
864880
struct fsck_options *options)
865881
{
866882
const char *p = *ident;
867883
const char *nl;
868-
char *end;
869884

870885
nl = memchr(p, '\n', ident_end - p);
871886
if (!nl)
@@ -917,11 +932,11 @@ static int fsck_ident(const char **ident, const char *ident_end,
917932
"invalid author/committer line - bad date");
918933
if (*p == '0' && p[1] != ' ')
919934
return report(options, oid, type, FSCK_MSG_ZERO_PADDED_DATE, "invalid author/committer line - zero-padded date");
920-
if (date_overflows(parse_timestamp(p, &end, 10)))
935+
if (date_overflows(parse_timestamp_from_buf(&p, ident_end)))
921936
return report(options, oid, type, FSCK_MSG_BAD_DATE_OVERFLOW, "invalid author/committer line - date causes integer overflow");
922-
if (*end != ' ')
937+
if (*p != ' ')
923938
return report(options, oid, type, FSCK_MSG_BAD_DATE, "invalid author/committer line - bad date");
924-
p = end + 1;
939+
p++;
925940
if ((*p != '+' && *p != '-') ||
926941
!isdigit(p[1]) ||
927942
!isdigit(p[2]) ||

0 commit comments

Comments
 (0)