[Outreachy][PATCH] fsck: use starts_with() in fsck_commit()

[keita <rudykeita@proton.me>]

From 30136adebaffb97edacae2c58c4ea491e39e3f5b Mon Sep 17 00:00:00 2001From: Songiso Cooper Lyambai <rudykeita@proton.me>
Date: Fri, 31 Oct 2025 23:45:23 +0200
Subject: [PATCH] fsck: use starts_with() in fsck_commit()

* Replace manual buffer checks with starts_with() for safety and clarity. 
*This avoids buffer overreads and follows Git's idiomatic style used 

--- fsck.c | 124 +++++++++++++++++++++++++++++++-------------------------- 1 file changed, 67 insertions(+), 57 deletions(-) diff --git a/fsck.c b/fsck.c index 341e100d24..7172c4ff1c 100644 --- a/fsck.c +++ b/fsck.c @@ -921,67 +921,77 @@ static int fsck_ident(const char **ident, } static int fsck_commit(const struct object_id *oid, - const char *buffer, unsigned long size, - struct fsck_options *options) + const char *buffer, unsigned long size, + struct fsck_options *options) { - struct object_id tree_oid, parent_oid; - unsigned author_count; - int err; - const char *buffer_begin = buffer; - const char *buffer_end = buffer + size; - const char *p; + struct object_id tree_oid, parent_oid; + unsigned author_count = 0; + int err = 0; + const char *buffer_end = buffer + size; + const char *p; - /* - * We _must_ stop parsing immediately if this reports failure, as the - * memory safety of the rest of the function depends on it. See the - * comment above the definition of verify_headers() for more details. - */ - if (verify_headers(buffer, size, oid, OBJ_COMMIT, options)) - return -1; - if (buffer >= buffer_end || !skip_prefix(buffer, "tree ", &buffer)) - return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_TREE, "invalid format - expected 'tree' line"); - if (parse_oid_hex(buffer, &tree_oid, &p) || *p != '\n') { - err = report(options, oid, OBJ_COMMIT, FSCK_MSG_BAD_TREE_SHA1, "invalid 'tree' line format - bad sha1"); - if (err) - return err; - } - buffer = p + 1; - while (buffer < buffer_end && skip_prefix(buffer, "parent ", &buffer)) { - if (parse_oid_hex(buffer, &parent_oid, &p) || *p != '\n') { - err = report(options, oid, OBJ_COMMIT, FSCK_MSG_BAD_PARENT_SHA1, "invalid 'parent' line format - bad sha1"); - if (err) - return err; - } - buffer = p + 1; - } - author_count = 0; - while (buffer < buffer_end && skip_prefix(buffer, "author ", &buffer)) { - author_count++; - err = fsck_ident(&buffer, oid, OBJ_COMMIT, options); - if (err) - return err; - } - if (author_count < 1) - err = report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_AUTHOR, "invalid format - expected 'author' line"); - else if (author_count > 1) - err = report(options, oid, OBJ_COMMIT, FSCK_MSG_MULTIPLE_AUTHORS, "invalid format - multiple 'author' lines"); - if (err) - return err; - if (buffer >= buffer_end || !skip_prefix(buffer, "committer ", &buffer)) - return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_COMMITTER, "invalid format - expected 'committer' line"); - err = fsck_ident(&buffer, oid, OBJ_COMMIT, options); - if (err) - return err; - if (memchr(buffer_begin, '\0', size)) { - err = report(options, oid, OBJ_COMMIT, FSCK_MSG_NUL_IN_COMMIT, - "NUL byte in the commit object body"); - if (err) - return err; - } - return 0; + /* + * We _must_ stop parsing immediately if this reports failure, as the + * memory safety of the rest of the function depends on it. See the + * comment above the definition of verify_headers() for more details. + */ + + if (verify_headers(buffer, size, oid, OBJ_COMMIT, options)) + return -1; + + + if (!skip_prefix(buffer, "tree ", &buffer)) + return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_TREE, + "invalid format - expected 'tree' line"); + if (parse_oid_hex(buffer, &tree_oid, &p) || *p != '\n') { + return report(options, oid, OBJ_COMMIT, FSCK_MSG_BAD_TREE_SHA1, + "invalid 'tree' line format - bad sha1"); + } + buffer = p + 1; + + while (starts_with(buffer, "parent ")) { + if (!skip_prefix(buffer, "parent ", &buffer) || + parse_oid_hex(buffer, &parent_oid, &p) || *p != '\n') { + return report(options, oid, OBJ_COMMIT, FSCK_MSG_BAD_PARENT_SHA1, + "invalid 'parent' line format - bad sha1"); + } + buffer = p + 1; + } + + while (starts_with(buffer, "author ")) { + author_count++; + if (!skip_prefix(buffer, "author ", &buffer)) + return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_AUTHOR, + "invalid format - expected 'author' line"); + if ((err = fsck_ident(&buffer, oid, OBJ_COMMIT, options))) + return err; + } + + if (author_count < 1) + return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_AUTHOR, + "invalid format - expected 'author' line"); + if (author_count > 1) + return report(options, oid, OBJ_COMMIT, FSCK_MSG_MULTIPLE_AUTHORS, + "invalid format - multiple 'author' lines"); + + if (!starts_with(buffer, "committer ")) + return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_COMMITTER, + "invalid format - expected 'committer' line"); + + if (!skip_prefix(buffer, "committer ", &buffer)) + return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_COMMITTER, + "invalid format - expected 'committer' line"); + + if ((err = fsck_ident(&buffer, oid, OBJ_COMMIT, options))) + return err; + + if (memchr(buffer, '\0', buffer_end - buffer)) + return report(options, oid, OBJ_COMMIT, FSCK_MSG_NUL_IN_COMMIT, + "NUL byte in the commit object body"); + + return 0; }




Signed-off-by: Songiso Cooper Lyambai <rudykeita@proton.me>