]> git.ipfire.org Git - thirdparty/git.git/blobdiff - fsck.c
t3428: restore coverage for "apply" backend
[thirdparty/git.git] / fsck.c
diff --git a/fsck.c b/fsck.c
index 47eaeedd7076ba60a621e072abb405127b3c33fe..2b18717ee805bc73aa50943731ac9a7212303d18 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -748,6 +748,23 @@ static int fsck_tree(const struct object_id *tree_oid,
        return retval;
 }
 
+/*
+ * Confirm that the headers of a commit or tag object end in a reasonable way,
+ * either with the usual "\n\n" separator, or at least with a trailing newline
+ * on the final header line.
+ *
+ * This property is important for the memory safety of our callers. It allows
+ * them to scan the buffer linewise without constantly checking the remaining
+ * size as long as:
+ *
+ *   - they check that there are bytes left in the buffer at the start of any
+ *     line (i.e., that the last newline they saw was not the final one we
+ *     found here)
+ *
+ *   - any intra-line scanning they do will stop at a newline, which will worst
+ *     case hit the newline we found here as the end-of-header. This makes it
+ *     OK for them to use helpers like parse_oid_hex(), or even skip_prefix().
+ */
 static int verify_headers(const void *data, unsigned long size,
                          const struct object_id *oid, enum object_type type,
                          struct fsck_options *options)
@@ -808,6 +825,20 @@ static int fsck_ident(const char **ident,
        if (*p != ' ')
                return report(options, oid, type, FSCK_MSG_MISSING_SPACE_BEFORE_DATE, "invalid author/committer line - missing space before date");
        p++;
+       /*
+        * Our timestamp parser is based on the C strto*() functions, which
+        * will happily eat whitespace, including the newline that is supposed
+        * to prevent us walking past the end of the buffer. So do our own
+        * scan, skipping linear whitespace but not newlines, and then
+        * confirming we found a digit. We _could_ be even more strict here,
+        * as we really expect only a single space, but since we have
+        * traditionally allowed extra whitespace, we'll continue to do so.
+        */
+       while (*p == ' ' || *p == '\t')
+               p++;
+       if (!isdigit(*p))
+               return report(options, oid, type, FSCK_MSG_BAD_DATE,
+                             "invalid author/committer line - bad date");
        if (*p == '0' && p[1] != ' ')
                return report(options, oid, type, FSCK_MSG_ZERO_PADDED_DATE, "invalid author/committer line - zero-padded date");
        if (date_overflows(parse_timestamp(p, &end, 10)))
@@ -834,12 +865,18 @@ static int fsck_commit(const struct object_id *oid,
        unsigned author_count;
        int err;
        const char *buffer_begin = buffer;
+       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 (!skip_prefix(buffer, "tree ", &buffer))
+       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");
@@ -847,7 +884,7 @@ static int fsck_commit(const struct object_id *oid,
                        return err;
        }
        buffer = p + 1;
-       while (skip_prefix(buffer, "parent ", &buffer)) {
+       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)
@@ -856,7 +893,7 @@ static int fsck_commit(const struct object_id *oid,
                buffer = p + 1;
        }
        author_count = 0;
-       while (skip_prefix(buffer, "author ", &buffer)) {
+       while (buffer < buffer_end && skip_prefix(buffer, "author ", &buffer)) {
                author_count++;
                err = fsck_ident(&buffer, oid, OBJ_COMMIT, options);
                if (err)
@@ -868,7 +905,7 @@ static int fsck_commit(const struct object_id *oid,
                err = report(options, oid, OBJ_COMMIT, FSCK_MSG_MULTIPLE_AUTHORS, "invalid format - multiple 'author' lines");
        if (err)
                return err;
-       if (!skip_prefix(buffer, "committer ", &buffer))
+       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)
@@ -899,13 +936,19 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        int ret = 0;
        char *eol;
        struct strbuf sb = STRBUF_INIT;
+       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.
+        */
        ret = verify_headers(buffer, size, oid, OBJ_TAG, options);
        if (ret)
                goto done;
 
-       if (!skip_prefix(buffer, "object ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "object ", &buffer)) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_OBJECT, "invalid format - expected 'object' line");
                goto done;
        }
@@ -916,11 +959,11 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        }
        buffer = p + 1;
 
-       if (!skip_prefix(buffer, "type ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "type ", &buffer)) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TYPE_ENTRY, "invalid format - expected 'type' line");
                goto done;
        }
-       eol = strchr(buffer, '\n');
+       eol = memchr(buffer, '\n', buffer_end - buffer);
        if (!eol) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TYPE, "invalid format - unexpected end after 'type' line");
                goto done;
@@ -932,11 +975,11 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
                goto done;
        buffer = eol + 1;
 
-       if (!skip_prefix(buffer, "tag ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "tag ", &buffer)) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TAG_ENTRY, "invalid format - expected 'tag' line");
                goto done;
        }
-       eol = strchr(buffer, '\n');
+       eol = memchr(buffer, '\n', buffer_end - buffer);
        if (!eol) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TAG, "invalid format - unexpected end after 'type' line");
                goto done;
@@ -952,7 +995,7 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        }
        buffer = eol + 1;
 
-       if (!skip_prefix(buffer, "tagger ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "tagger ", &buffer)) {
                /* early tags do not contain 'tagger' lines; warn only */
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TAGGER_ENTRY, "invalid format - expected 'tagger' line");
                if (ret)
@@ -960,10 +1003,8 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        }
        else
                ret = fsck_ident(&buffer, oid, OBJ_TAG, options);
-       if (!*buffer)
-               goto done;
 
-       if (!starts_with(buffer, "\n")) {
+       if (buffer < buffer_end && !starts_with(buffer, "\n")) {
                /*
                 * The verify_headers() check will allow
                 * e.g. "[...]tagger <tagger>\nsome
@@ -1237,19 +1278,26 @@ int fsck_object(struct object *obj, void *data, unsigned long size,
        if (!obj)
                return report(options, NULL, OBJ_NONE, FSCK_MSG_BAD_OBJECT_SHA1, "no valid object to fsck");
 
-       if (obj->type == OBJ_BLOB)
-               return fsck_blob(&obj->oid, data, size, options);
-       if (obj->type == OBJ_TREE)
-               return fsck_tree(&obj->oid, data, size, options);
-       if (obj->type == OBJ_COMMIT)
-               return fsck_commit(&obj->oid, data, size, options);
-       if (obj->type == OBJ_TAG)
-               return fsck_tag(&obj->oid, data, size, options);
+       return fsck_buffer(&obj->oid, obj->type, data, size, options);
+}
+
+int fsck_buffer(const struct object_id *oid, enum object_type type,
+               void *data, unsigned long size,
+               struct fsck_options *options)
+{
+       if (type == OBJ_BLOB)
+               return fsck_blob(oid, data, size, options);
+       if (type == OBJ_TREE)
+               return fsck_tree(oid, data, size, options);
+       if (type == OBJ_COMMIT)
+               return fsck_commit(oid, data, size, options);
+       if (type == OBJ_TAG)
+               return fsck_tag(oid, data, size, options);
 
-       return report(options, &obj->oid, obj->type,
+       return report(options, oid, type,
                      FSCK_MSG_UNKNOWN_TYPE,
                      "unknown type '%d' (internal fsck error)",
-                     obj->type);
+                     type);
 }
 
 int fsck_error_function(struct fsck_options *o,