From: keita Date: Fri, 31 Oct 2025 22:00:27 GMT Subject: [PATCH] fsck: use starts_with() in fsck_commit() Message-ID: From 30136adebaffb97edacae2c58c4ea491e39e3f5b Mon Sep 17 00:00:00 2001From: Songiso Cooper Lyambai 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  Signed-off-by: Songiso Cooper Lyambai ---  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;  }