[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 


Signed-off-by: Songiso Cooper Lyambai <rudykeita@proton.me>
---
 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;
 }