@matheusbsbs / matheus-bagliano / commits / dd8e8c786e

submodule add: sanity check existing .gitmodules

"git submodule add" tries to find if a submodule with the same name already exists at a different path, by looking up an entry in the .gitmodules file. If the entry in the file is incomplete, e.g., when the submodule.<name>.something variable is defined but there is no definition of submodule.<name>.path variable, it accesses the missing .path member of the submodule structure and triggers a segfault. A brief audit was done to make sure that the code does not assume members other than those that are absolutely certain to exist: a submodule obtained by submodule_from_name() should have .name member, while a submodule obtained by submodule_from_path() should also have .path as well as .name member, and we cannot assume anything else. Luckily, the module_add() codepath was the only problematic one. It is fairly recent code that comes from 1fa06ced (submodule: prevent overwriting .gitmodules on path reuse, 2025-07-24). A helper used by update_submodule() seems to assume that its call to submodule_from_path() always yields a submodule object without a failure, which seems to rely on the caller making sure it is the case. Leave an assert() with a NEEDSWORK comment there for future developers to make sure the assumption actually holds. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Nov 15, 2025 at 23:02 UTC dd8e8c786efdfb3ba588d807bfb0dc0d5196c343
2 files changed +29 -2
builtin/submodule--helper.c
+10 -2
@@ -1913,6 +1913,13 @@ static int determine_submodule_update_strategy(struct repository *r,
1913 const char *val;
1914 int ret;
1915
1916 + /*
1917 + * NEEDSWORK: audit and ensure that update_submodule() has right
1918 + * to assume that submodule_from_path() above will always succeed.
1919 + */
1920 + if (!sub)
1921 + BUG("update_submodule assumes a submodule exists at path (%s)",
1922 + path);
1923 key = xstrfmt("submodule.%s.update", sub->name);
1924
1925 if (update) {
@@ -3537,14 +3544,15 @@ static int module_add(int argc, const char **argv, const char *prefix,
3544 }
3545 }
3546
3540 - if(!add_data.sm_name)
3547 + if (!add_data.sm_name)
3548 add_data.sm_name = add_data.sm_path;
3549
3550 existing = submodule_from_name(the_repository,
3551 null_oid(the_hash_algo),
3552 add_data.sm_name);
3553
3547 - if (existing && strcmp(existing->path, add_data.sm_path)) {
3554 + if (existing && existing->path &&
3555 + strcmp(existing->path, add_data.sm_path)) {
3556 if (!force) {
3557 die(_("submodule name '%s' already used for path '%s'"),
3558 add_data.sm_name, existing->path);
t/t7400-submodule-basic.sh
+19
@@ -48,6 +48,25 @@ test_expect_success 'submodule deinit works on empty repository' '
48 git submodule deinit --all
49 '
50
51 +test_expect_success 'submodule add with incomplete .gitmodules' '
52 + test_when_finished "rm -f expect actual" &&
53 + test_when_finished "git config remove-section submodule.one" &&
54 + test_when_finished "git rm -f one .gitmodules" &&
55 + git init one &&
56 + git -C one commit --allow-empty -m one-initial &&
57 + git config -f .gitmodules submodule.one.ignore all &&
58 +
59 + git submodule add ./one &&
60 +
61 + for var in ignore path url
62 + do
63 + git config -f .gitmodules --get "submodule.one.$var" ||
64 + return 1
65 + done >actual &&
66 + test_write_lines all one ./one >expect &&
67 + test_cmp expect actual
68 +'
69 +
70 test_expect_success 'setup - initial commit' '
71 >t &&
72 git add t &&