Skip to content

Commit 1a7fd1f

Browse files
peffgitster
authored andcommitted
fsck: detect submodule paths starting with dash
As with urls, submodule paths with dashes are ignored by git, but may end up confusing older versions. Detecting them via fsck lets us prevent modern versions of git from being a vector to spread broken .gitmodules to older versions. Compared to blocking leading-dash urls, though, this detection may be less of a good idea: 1. While such paths provide confusing and broken results, they don't seem to actually work as option injections against anything except "cd". In particular, the submodule code seems to canonicalize to an absolute path before running "git clone" (so it passes /your/clone/-sub). 2. It's more likely that we may one day make such names actually work correctly. Even after we revert this fsck check, it will continue to be a hassle until hosting servers are all updated. On the other hand, it's not entirely clear that the behavior in older versions is safe. And if we do want to eventually allow this, we may end up doing so with a special syntax anyway (e.g., writing "./-sub" in the .gitmodules file, and teaching the submodule code to canonicalize it when comparing). So on balance, this is probably a good protection. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a124133 commit 1a7fd1f

File tree

2 files changed

+15
-0
lines changed

2 files changed

+15
-0
lines changed

fsck.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ static struct oidset gitmodules_done = OIDSET_INIT;
6565
FUNC(GITMODULES_NAME, ERROR) \
6666
FUNC(GITMODULES_SYMLINK, ERROR) \
6767
FUNC(GITMODULES_URL, ERROR) \
68+
FUNC(GITMODULES_PATH, ERROR) \
6869
/* warnings */ \
6970
FUNC(BAD_FILEMODE, WARN) \
7071
FUNC(EMPTY_NAME, WARN) \
@@ -952,6 +953,12 @@ static int fsck_gitmodules_fn(const char *var, const char *value, void *vdata)
952953
FSCK_MSG_GITMODULES_URL,
953954
"disallowed submodule url: %s",
954955
value);
956+
if (!strcmp(key, "path") && value &&
957+
looks_like_command_line_option(value))
958+
data->ret |= report(data->options, data->obj,
959+
FSCK_MSG_GITMODULES_PATH,
960+
"disallowed submodule path: %s",
961+
value);
955962
free(name);
956963

957964
return 0;

t/t7417-submodule-path-url.sh

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,12 @@ test_expect_success 'clone rejects unprotected dash' '
1717
test_i18ngrep ignoring err
1818
'
1919

20+
test_expect_success 'fsck rejects unprotected dash' '
21+
test_when_finished "rm -rf dst" &&
22+
git init --bare dst &&
23+
git -C dst config transfer.fsckObjects true &&
24+
test_must_fail git push dst HEAD 2>err &&
25+
grep gitmodulesPath err
26+
'
27+
2028
test_done

0 commit comments

Comments
 (0)