@matheusbsbs / matheus-bagliano / commits / 27ab4784d5

fsck: implement checks for gitattributes

Recently, a vulnerability was reported that can lead to an out-of-bounds write when reading an unreasonably large gitattributes file. The root cause of this error are multiple integer overflows in different parts of the code when there are either too many lines, when paths are too long, when attribute names are too long, or when there are too many attributes declared for a pattern. As all of these are related to size, it seems reasonable to restrict the size of the gitattributes file via git-fsck(1). This allows us to both stop distributing known-vulnerable objects via common hosting platforms that have fsck enabled, and users to protect themselves by enabling the `fetch.fsckObjects` config. There are basically two checks: 1. We verify that size of the gitattributes file is smaller than 100MB. 2. We verify that the maximum line length does not exceed 2048 bytes. With the preceding commits, both of these conditions would cause us to either ignore the complete gitattributes file or blob in the first case, or the specific line in the second case. Now with these consistency checks added, we also grow the ability to stop distributing such files in the first place when `receive.fsckObjects` is enabled. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Dec 1, 2022 at 15:46 UTC 27ab4784d5c9e24345b9f5b443609cbe527c51f9

3 files changed +73 -1

fsck.c

+37 -1

@@ -2,6 +2,7 @@

2 #include "object-store.h"

3 #include "repository.h"

4 #include "object.h"

5 +#include "attr.h"

6 #include "blob.h"

7 #include "tree.h"

8 #include "tree-walk.h"

@@ -615,7 +616,10 @@ static int fsck_tree(const struct object_id *tree_oid,

616 }

617

618 if (is_hfs_dotgitattributes(name) || is_ntfs_dotgitattributes(name)) {

618 - if (S_ISLNK(mode))

619 + if (!S_ISLNK(mode))

620 + oidset_insert(&options->gitattributes_found,

621 + entry_oid);

622 + else

623 retval += report(options, tree_oid, OBJ_TREE,

624 FSCK_MSG_GITATTRIBUTES_SYMLINK,

625 ".gitattributes is a symlink");

@@ -1206,6 +1210,35 @@ static int fsck_blob(const struct object_id *oid, const char *buf,

1210 ret |= data.ret;

1211 }

1212

1213 + if (oidset_contains(&options->gitattributes_found, oid)) {

1214 + const char *ptr;

1215 +

1216 + oidset_insert(&options->gitattributes_done, oid);

1217 +

1218 + if (!buf || size > ATTR_MAX_FILE_SIZE) {

1219 + /*

1220 + * A missing buffer here is a sign that the caller found the

1221 + * blob too gigantic to load into memory. Let's just consider

1222 + * that an error.

1223 + */

1224 + return report(options, oid, OBJ_BLOB,

1225 + FSCK_MSG_GITATTRIBUTES_LARGE,

1226 + ".gitattributes too large to parse");

1227 + }

1228 +

1229 + for (ptr = buf; *ptr; ) {

1230 + const char *eol = strchrnul(ptr, '\n');

1231 + if (eol - ptr >= ATTR_MAX_LINE_LENGTH) {

1232 + ret |= report(options, oid, OBJ_BLOB,

1233 + FSCK_MSG_GITATTRIBUTES_LINE_LENGTH,

1234 + ".gitattributes has too long lines to parse");

1235 + break;

1236 + }

1237 +

1238 + ptr = *eol ? eol + 1 : eol;

1239 + }

1240 + }

1241 +

1242 return ret;

1243 }

1244

@@ -1293,6 +1326,9 @@ int fsck_finish(struct fsck_options *options)

1326 ret |= fsck_blobs(&options->gitmodules_found, &options->gitmodules_done,

1327 FSCK_MSG_GITMODULES_MISSING, FSCK_MSG_GITMODULES_BLOB,

1328 options, ".gitmodules");

1329 + ret |= fsck_blobs(&options->gitattributes_found, &options->gitattributes_done,

1330 + FSCK_MSG_GITATTRIBUTES_MISSING, FSCK_MSG_GITATTRIBUTES_BLOB,

1331 + options, ".gitattributes");

1332

1333 return ret;

1334 }

fsck.h

+12

@@ -55,6 +55,10 @@ enum fsck_msg_type {

55 FUNC(GITMODULES_URL, ERROR) \

56 FUNC(GITMODULES_PATH, ERROR) \

57 FUNC(GITMODULES_UPDATE, ERROR) \

58 + FUNC(GITATTRIBUTES_MISSING, ERROR) \

59 + FUNC(GITATTRIBUTES_LARGE, ERROR) \

60 + FUNC(GITATTRIBUTES_LINE_LENGTH, ERROR) \

61 + FUNC(GITATTRIBUTES_BLOB, ERROR) \

62 /* warnings */ \

63 FUNC(BAD_FILEMODE, WARN) \

64 FUNC(EMPTY_NAME, WARN) \

@@ -129,6 +133,8 @@ struct fsck_options {

133 struct oidset skiplist;

134 struct oidset gitmodules_found;

135 struct oidset gitmodules_done;

136 + struct oidset gitattributes_found;

137 + struct oidset gitattributes_done;

138 kh_oid_map_t *object_names;

139 };

140

@@ -136,18 +142,24 @@ struct fsck_options {

142 .skiplist = OIDSET_INIT, \

143 .gitmodules_found = OIDSET_INIT, \

144 .gitmodules_done = OIDSET_INIT, \

145 + .gitattributes_found = OIDSET_INIT, \

146 + .gitattributes_done = OIDSET_INIT, \

147 .error_func = fsck_error_function \

148 }

149 #define FSCK_OPTIONS_STRICT { \

150 .strict = 1, \

151 .gitmodules_found = OIDSET_INIT, \

152 .gitmodules_done = OIDSET_INIT, \

153 + .gitattributes_found = OIDSET_INIT, \

154 + .gitattributes_done = OIDSET_INIT, \

155 .error_func = fsck_error_function, \

156 }

157 #define FSCK_OPTIONS_MISSING_GITMODULES { \

158 .strict = 1, \

159 .gitmodules_found = OIDSET_INIT, \

160 .gitmodules_done = OIDSET_INIT, \

161 + .gitattributes_found = OIDSET_INIT, \

162 + .gitattributes_done = OIDSET_INIT, \

163 .error_func = fsck_error_cb_print_missing_gitmodules, \

164 }

165

t/t1450-fsck.sh

+24

@@ -865,4 +865,28 @@ test_expect_success 'detect corrupt index file in fsck' '

865 test_i18ngrep "bad index file" errors

866 '

867

868 +test_expect_success 'fsck error on gitattributes with excessive line lengths' '

869 + blob=$(printf "pattern %02048d" 1 | git hash-object -w --stdin) &&

870 + test_when_finished "remove_object $blob" &&

871 + tree=$(printf "100644 blob %s\t%s\n" $blob .gitattributes | git mktree) &&

872 + test_when_finished "remove_object $tree" &&

873 + cat >expected <<-EOF &&

874 + error in blob $blob: gitattributesLineLength: .gitattributes has too long lines to parse

875 + EOF

876 + test_must_fail git fsck --no-dangling >actual 2>&1 &&

877 + test_cmp expected actual

878 +'

879 +

880 +test_expect_success 'fsck error on gitattributes with excessive size' '

881 + blob=$(test-tool genzeros $((100 * 1024 * 1024 + 1)) | git hash-object -w --stdin) &&

882 + test_when_finished "remove_object $blob" &&

883 + tree=$(printf "100644 blob %s\t%s\n" $blob .gitattributes | git mktree) &&

884 + test_when_finished "remove_object $tree" &&

885 + cat >expected <<-EOF &&

886 + error in blob $blob: gitattributesLarge: .gitattributes too large to parse

887 + EOF

888 + test_must_fail git fsck --no-dangling >actual 2>&1 &&

889 + test_cmp expected actual

890 +'

891 +

892 test_done