@matheusbsbs / matheus-bagliano / commits / 0b6ec075df

fsck: assert newline presence in fsck_ident()

The fsck code purports to handle buffers that are not NUL-terminated, but fsck_ident() uses some string functions. This works OK in practice, as explained in 8e4309038f (fsck: do not assume NUL-termination of buffers, 2023-01-19). Before calling fsck_ident() we'll have called verify_headers(), which makes sure we have at least a trailing newline. And none of our string-like functions will walk past that newline. However, that makes this code at the top of fsck_ident() very confusing: *ident = strchrnul(*ident, '\n'); if (**ident == '\n') (*ident)++; We should always see that newline, or our memory safety assumptions have been violated! Further, using strchrnul() is weird, since the whole point is that if the newline is not there, we don't necessarily have a NUL at all, and might read off the end of the buffer. So let's have callers pass in the boundary of our buffer, which lets us safely find the newline with memchr(). And if it is not there, this is a BUG(), because it means our caller did not validate the input with verify_headers() as it was supposed to (and we are better off bailing rather than having memory-safety problems). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Nov 18, 2025 at 04:12 UTC 0b6ec075df2ac77a4792b8b1a7290a36b636012b

1 file changed +9 -7

fsck.c

+9 -7

@@ -859,16 +859,18 @@ static int verify_headers(const void *data, unsigned long size,

859 FSCK_MSG_UNTERMINATED_HEADER, "unterminated header");

860 }

861

862 -static int fsck_ident(const char **ident,

862 +static int fsck_ident(const char **ident, const char *ident_end,

863 const struct object_id *oid, enum object_type type,

864 struct fsck_options *options)

865 {

866 const char *p = *ident;

867 + const char *nl;

868 char *end;

869

869 - *ident = strchrnul(*ident, '\n');

870 - if (**ident == '\n')

871 - (*ident)++;

870 + nl = memchr(p, '\n', ident_end - p);

871 + if (!nl)

872 + BUG("verify_headers() should have made sure we have a newline");

873 + *ident = nl + 1;

874

875 if (*p == '<')

876 return report(options, oid, type, FSCK_MSG_MISSING_NAME_BEFORE_EMAIL, "invalid author/committer line - missing space before email");

@@ -957,7 +959,7 @@ static int fsck_commit(const struct object_id *oid,

959 author_count = 0;

960 while (buffer < buffer_end && skip_prefix(buffer, "author ", &buffer)) {

961 author_count++;

960 - err = fsck_ident(&buffer, oid, OBJ_COMMIT, options);

962 + err = fsck_ident(&buffer, buffer_end, oid, OBJ_COMMIT, options);

963 if (err)

964 return err;

965 }

@@ -969,7 +971,7 @@ static int fsck_commit(const struct object_id *oid,

971 return err;

972 if (buffer >= buffer_end || !skip_prefix(buffer, "committer ", &buffer))

973 return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_COMMITTER, "invalid format - expected 'committer' line");

972 - err = fsck_ident(&buffer, oid, OBJ_COMMIT, options);

974 + err = fsck_ident(&buffer, buffer_end, oid, OBJ_COMMIT, options);

975 if (err)

976 return err;

977 if (memchr(buffer_begin, '\0', size)) {

@@ -1064,7 +1066,7 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,

1066 goto done;

1067 }

1068 else

1067 - ret = fsck_ident(&buffer, oid, OBJ_TAG, options);

1069 + ret = fsck_ident(&buffer, buffer_end, oid, OBJ_TAG, options);

1070

1071 if (buffer < buffer_end && !starts_with(buffer, "\n")) {

1072 /*