fsck: warn about symlink pointing inside a gitdir
In the wake of fixing a vulnerability where `git clone` mistakenly followed a symbolic link that it had just written while checking out files, writing into a gitdir, let's add some defense-in-depth by teaching `git fsck` to report symbolic links stored in its trees that point inside `.git/`. Even though the Git project never made any promises about the exact shape of the `.git/` directory's contents, there are likely repositories out there containing symbolic links that point inside the gitdir. For that reason, let's only report these as warnings, not as errors. Security-conscious users are encouraged to configure `fsck.symlinkPointsToGitDir = error`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Johannes Schindelin committed
Apr 10, 2024 at 18:01 UTC
a33fea0886cfa016d313d2bd66bdd08615bffbc9
4 files changed
+117
Documentation/fsck-msgids.txt
+12
@@ -157,6 +157,18 @@
157
`nullSha1`::
158
(WARN) Tree contains entries pointing to a null sha1.
159
160
+`symlinkPointsToGitDir`::
161
+ (WARN) Symbolic link points inside a gitdir.
162
+
163
+`symlinkTargetBlob`::
164
+ (ERROR) A non-blob found instead of a symbolic link's target.
165
+
166
+`symlinkTargetLength`::
167
+ (WARN) Symbolic link target longer than maximum path length.
168
+
169
+`symlinkTargetMissing`::
170
+ (ERROR) Unable to read symbolic link target's blob.
171
+
172
`treeNotSorted`::
173
(ERROR) A tree is not properly sorted.
174
fsck.c
+56
@@ -636,6 +636,8 @@ static int fsck_tree(const struct object_id *tree_oid,
636
retval += report(options, tree_oid, OBJ_TREE,
637
FSCK_MSG_MAILMAP_SYMLINK,
638
".mailmap is a symlink");
639
+ oidset_insert(&options->symlink_targets_found,
640
+ entry_oid);
641
}
642
643
if ((backslash = strchr(name, '\\'))) {
@@ -1228,6 +1230,56 @@ static int fsck_blob(const struct object_id *oid, const char *buf,
1230
}
1231
}
1232
1233
+ if (oidset_contains(&options->symlink_targets_found, oid)) {
1234
+ const char *ptr = buf;
1235
+ const struct object_id *reported = NULL;
1236
+
1237
+ oidset_insert(&options->symlink_targets_done, oid);
1238
+
1239
+ if (!buf || size > PATH_MAX) {
1240
+ /*
1241
+ * A missing buffer here is a sign that the caller found the
1242
+ * blob too gigantic to load into memory. Let's just consider
1243
+ * that an error.
1244
+ */
1245
+ return report(options, oid, OBJ_BLOB,
1246
+ FSCK_MSG_SYMLINK_TARGET_LENGTH,
1247
+ "symlink target too long");
1248
+ }
1249
+
1250
+ while (!reported && ptr) {
1251
+ const char *p = ptr;
1252
+ char c, *slash = strchrnul(ptr, '/');
1253
+ char *backslash = memchr(ptr, '\\', slash - ptr);
1254
+
1255
+ c = *slash;
1256
+ *slash = '\0';
1257
+
1258
+ while (!reported && backslash) {
1259
+ *backslash = '\0';
1260
+ if (is_ntfs_dotgit(p))
1261
+ ret |= report(options, reported = oid, OBJ_BLOB,
1262
+ FSCK_MSG_SYMLINK_POINTS_TO_GIT_DIR,
1263
+ "symlink target points to git dir");
1264
+ *backslash = '\\';
1265
+ p = backslash + 1;
1266
+ backslash = memchr(p, '\\', slash - p);
1267
+ }
1268
+ if (!reported && is_ntfs_dotgit(p))
1269
+ ret |= report(options, reported = oid, OBJ_BLOB,
1270
+ FSCK_MSG_SYMLINK_POINTS_TO_GIT_DIR,
1271
+ "symlink target points to git dir");
1272
+
1273
+ if (!reported && is_hfs_dotgit(ptr))
1274
+ ret |= report(options, reported = oid, OBJ_BLOB,
1275
+ FSCK_MSG_SYMLINK_POINTS_TO_GIT_DIR,
1276
+ "symlink target points to git dir");
1277
+
1278
+ *slash = c;
1279
+ ptr = c ? slash + 1 : NULL;
1280
+ }
1281
+ }
1282
+
1283
return ret;
1284
}
1285
@@ -1319,6 +1371,10 @@ int fsck_finish(struct fsck_options *options)
1371
FSCK_MSG_GITATTRIBUTES_MISSING, FSCK_MSG_GITATTRIBUTES_BLOB,
1372
options, ".gitattributes");
1373
1374
+ ret |= fsck_blobs(&options->symlink_targets_found, &options->symlink_targets_done,
1375
+ FSCK_MSG_SYMLINK_TARGET_MISSING, FSCK_MSG_SYMLINK_TARGET_BLOB,
1376
+ options, "<symlink-target>");
1377
+
1378
return ret;
1379
}
1380
fsck.h
+12
@@ -63,6 +63,8 @@ enum fsck_msg_type {
63
FUNC(GITATTRIBUTES_LARGE, ERROR) \
64
FUNC(GITATTRIBUTES_LINE_LENGTH, ERROR) \
65
FUNC(GITATTRIBUTES_BLOB, ERROR) \
66
+ FUNC(SYMLINK_TARGET_MISSING, ERROR) \
67
+ FUNC(SYMLINK_TARGET_BLOB, ERROR) \
68
/* warnings */ \
69
FUNC(EMPTY_NAME, WARN) \
70
FUNC(FULL_PATHNAME, WARN) \
@@ -72,6 +74,8 @@ enum fsck_msg_type {
74
FUNC(NULL_SHA1, WARN) \
75
FUNC(ZERO_PADDED_FILEMODE, WARN) \
76
FUNC(NUL_IN_COMMIT, WARN) \
77
+ FUNC(SYMLINK_TARGET_LENGTH, WARN) \
78
+ FUNC(SYMLINK_POINTS_TO_GIT_DIR, WARN) \
79
/* infos (reported as warnings, but ignored by default) */ \
80
FUNC(BAD_FILEMODE, INFO) \
81
FUNC(GITMODULES_PARSE, INFO) \
@@ -139,6 +143,8 @@ struct fsck_options {
143
struct oidset gitmodules_done;
144
struct oidset gitattributes_found;
145
struct oidset gitattributes_done;
146
+ struct oidset symlink_targets_found;
147
+ struct oidset symlink_targets_done;
148
kh_oid_map_t *object_names;
149
};
150
@@ -148,6 +154,8 @@ struct fsck_options {
154
.gitmodules_done = OIDSET_INIT, \
155
.gitattributes_found = OIDSET_INIT, \
156
.gitattributes_done = OIDSET_INIT, \
157
+ .symlink_targets_found = OIDSET_INIT, \
158
+ .symlink_targets_done = OIDSET_INIT, \
159
.error_func = fsck_error_function \
160
}
161
#define FSCK_OPTIONS_STRICT { \
@@ -156,6 +164,8 @@ struct fsck_options {
164
.gitmodules_done = OIDSET_INIT, \
165
.gitattributes_found = OIDSET_INIT, \
166
.gitattributes_done = OIDSET_INIT, \
167
+ .symlink_targets_found = OIDSET_INIT, \
168
+ .symlink_targets_done = OIDSET_INIT, \
169
.error_func = fsck_error_function, \
170
}
171
#define FSCK_OPTIONS_MISSING_GITMODULES { \
@@ -164,6 +174,8 @@ struct fsck_options {
174
.gitmodules_done = OIDSET_INIT, \
175
.gitattributes_found = OIDSET_INIT, \
176
.gitattributes_done = OIDSET_INIT, \
177
+ .symlink_targets_found = OIDSET_INIT, \
178
+ .symlink_targets_done = OIDSET_INIT, \
179
.error_func = fsck_error_cb_print_missing_gitmodules, \
180
}
181
t/t1450-fsck.sh
+37
@@ -1023,4 +1023,41 @@ test_expect_success 'fsck error on gitattributes with excessive size' '
1023
test_cmp expected actual
1024
'
1025
1026
+test_expect_success 'fsck warning on symlink target with excessive length' '
1027
+ symlink_target=$(printf "pattern %032769d" 1 | git hash-object -w --stdin) &&
1028
+ test_when_finished "remove_object $symlink_target" &&
1029
+ tree=$(printf "120000 blob %s\t%s\n" $symlink_target symlink | git mktree) &&
1030
+ test_when_finished "remove_object $tree" &&
1031
+ cat >expected <<-EOF &&
1032
+ warning in blob $symlink_target: symlinkTargetLength: symlink target too long
1033
+ EOF
1034
+ git fsck --no-dangling >actual 2>&1 &&
1035
+ test_cmp expected actual
1036
+'
1037
+
1038
+test_expect_success 'fsck warning on symlink target pointing inside git dir' '
1039
+ gitdir=$(printf ".git" | git hash-object -w --stdin) &&
1040
+ ntfs_gitdir=$(printf "GIT~1" | git hash-object -w --stdin) &&
1041
+ hfs_gitdir=$(printf ".${u200c}git" | git hash-object -w --stdin) &&
1042
+ inside_gitdir=$(printf "nested/.git/config" | git hash-object -w --stdin) &&
1043
+ benign_target=$(printf "legit/config" | git hash-object -w --stdin) &&
1044
+ tree=$(printf "120000 blob %s\t%s\n" \
1045
+ $benign_target benign_target \
1046
+ $gitdir gitdir \
1047
+ $hfs_gitdir hfs_gitdir \
1048
+ $inside_gitdir inside_gitdir \
1049
+ $ntfs_gitdir ntfs_gitdir |
1050
+ git mktree) &&
1051
+ for o in $gitdir $ntfs_gitdir $hfs_gitdir $inside_gitdir $benign_target $tree
1052
+ do
1053
+ test_when_finished "remove_object $o" || return 1
1054
+ done &&
1055
+ printf "warning in blob %s: symlinkPointsToGitDir: symlink target points to git dir\n" \
1056
+ $gitdir $hfs_gitdir $inside_gitdir $ntfs_gitdir |
1057
+ sort >expected &&
1058
+ git fsck --no-dangling >actual 2>&1 &&
1059
+ sort actual >actual.sorted &&
1060
+ test_cmp expected actual.sorted
1061
+'
1062
+
1063
test_done