aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2025-11-18 04:12:23 -0500
committerJunio C Hamano <gitster@pobox.com>2025-11-18 09:36:11 -0800
commit830424def4896dbf041d41dad873f5b86fdf9bfa (patch)
treea853590926e3a471ca1efc0b400dad47bfdf9487
parent0b6ec075df2ac77a4792b8b1a7290a36b636012b (diff)
downloadgit-830424def4896dbf041d41dad873f5b86fdf9bfa.tar.gz
fsck: avoid strcspn() in fsck_ident()
We may be operating on a buffer that is not NUL-terminated, but we use strcspn() to parse it. This is OK in practice, as discussed in 8e4309038f (fsck: do not assume NUL-termination of buffers, 2023-01-19), because we know there is at least a trailing newline in our buffer, and we always pass "\n" to strcspn(). So we know it will stop before running off the end of the buffer. But this is a subtle point to hang our memory safety hat on. And it confuses ASan's strict_string_checks mode, even though it is technically a false positive (that mode complains that we have no NUL, which is true, but it does not know that we have verified the presence of the newline already). Let's instead open-code the loop. As a bonus, this makes the logic more obvious (to my mind, anyway). The current code skips forward with strcspn until it hits "<", ">", or "\n". But then it must check which it saw to decide if that was what we expected or not, duplicating some logic between what's in the strcspn() and what's in the domain logic. Instead, we can just check each character as we loop and act on it immediately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--fsck.c32
1 files changed, 22 insertions, 10 deletions
diff --git a/fsck.c b/fsck.c
index 3d1027dc87..3696f1b849 100644
--- a/fsck.c
+++ b/fsck.c
@@ -874,18 +874,30 @@ static int fsck_ident(const char **ident, const char *ident_end,
if (*p == '<')
return report(options, oid, type, FSCK_MSG_MISSING_NAME_BEFORE_EMAIL, "invalid author/committer line - missing space before email");
- p += strcspn(p, "<>\n");
- if (*p == '>')
- return report(options, oid, type, FSCK_MSG_BAD_NAME, "invalid author/committer line - bad name");
- if (*p != '<')
- return report(options, oid, type, FSCK_MSG_MISSING_EMAIL, "invalid author/committer line - missing email");
+ for (;;) {
+ if (p >= ident_end || *p == '\n')
+ return report(options, oid, type, FSCK_MSG_MISSING_EMAIL, "invalid author/committer line - missing email");
+ if (*p == '>')
+ return report(options, oid, type, FSCK_MSG_BAD_NAME, "invalid author/committer line - bad name");
+ if (*p == '<')
+ break; /* end of name, beginning of email */
+
+ /* otherwise, skip past arbitrary name char */
+ p++;
+ }
if (p[-1] != ' ')
return report(options, oid, type, FSCK_MSG_MISSING_SPACE_BEFORE_EMAIL, "invalid author/committer line - missing space before email");
- p++;
- p += strcspn(p, "<>\n");
- if (*p != '>')
- return report(options, oid, type, FSCK_MSG_BAD_EMAIL, "invalid author/committer line - bad email");
- p++;
+ p++; /* skip past '<' we found */
+ for (;;) {
+ if (p >= ident_end || *p == '<' || *p == '\n')
+ return report(options, oid, type, FSCK_MSG_BAD_EMAIL, "invalid author/committer line - bad email");
+ if (*p == '>')
+ break; /* end of email */
+
+ /* otherwise, skip past arbitrary email char */
+ p++;
+ }
+ p++; /* skip past '>' we found */
if (*p != ' ')
return report(options, oid, type, FSCK_MSG_MISSING_SPACE_BEFORE_DATE, "invalid author/committer line - missing space before date");
p++;