init.templateDir: consider this config setting protected

The ability to configuring the template directory is a delicate feature:
It allows defining hooks that will be run e.g. during a `git clone`
operation, such as the `post-checkout` hook.

As such, it is of utmost importance that Git would not allow that config
setting to be changed during a `git clone` by mistake, allowing an
attacker a chance for a Remote Code Execution, allowing attackers to run
arbitrary code on unsuspecting users' machines.

As a defense-in-depth measure, to prevent minor vulnerabilities in the
`git clone` code from ballooning into higher-serverity attack vectors,
let's make this a protected setting just like `safe.directory` and
friends, i.e. ignore any `init.templateDir` entries from any local
config.

Note: This does not change the behavior of any recursive clone (modulo
bugs), as the local repository config is not even supposed to be written
while cloning the superproject, except in one scenario: If a config
template is configured that sets the template directory. This might be
done because `git clone --recurse-submodules --template=<directory>`
does not pass that template directory on to the submodules'
initialization.

Another scenario where this commit changes behavior is where
repositories are _not_ cloned recursively, and then some (intentional,
benign) automation configures the template directory to be used before
initializing the submodules.

So the caveat is that this could theoretically break existing processes.

In both scenarios, there is a way out, though: configuring the template
directory via the environment variable `GIT_TEMPLATE_DIR`.

This change in behavior is a trade-off between security and
backwards-compatibility that is struck in favor of security.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

diff --git a/setup.c b/setup.c
index e6e749ec4b4d848f6e4264de035e83c04d0185bc..c3301f5ab824b5c7d8d12c835dd2819368c6ffd0 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1726,6 +1726,31 @@ int daemonize(void)
 #define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
 #endif
 
+struct template_dir_cb_data {
+       char *path;
+       int initialized;
+};
+
+static int template_dir_cb(const char *key, const char *value, void *d)
+{
+       struct template_dir_cb_data *data = d;
+
+       if (strcmp(key, "init.templatedir"))
+               return 0;
+
+       if (!value) {
+               data->path = NULL;
+       } else {
+               char *path = NULL;
+
+               FREE_AND_NULL(data->path);
+               if (!git_config_pathname((const char **)&path, key, value))
+                       data->path = path ? path : xstrdup(value);
+       }
+
+       return 0;
+}
+
 const char *get_template_dir(const char *option_template)
 {
        const char *template_dir = option_template;
@@ -1733,15 +1758,13 @@ const char *get_template_dir(const char *option_template)
        if (!template_dir)
                template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
        if (!template_dir) {
-               static const char *init_template_dir;
-               static int initialized;
+               static struct template_dir_cb_data data;
 
-               if (!initialized) {
-                       git_config_get_pathname("init.templatedir",
-                                               &init_template_dir);
-                       initialized = 1;
+               if (!data.initialized) {
+                       git_protected_config(template_dir_cb, &data);
+                       data.initialized = 1;
                }
-               template_dir = init_template_dir;
+               template_dir = data.path;
        }
        if (!template_dir) {
                static char *dir;

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index eae6a46ef3d89f2563d5ec69f83dfd2ad6e7829f..3e8cf9b88567fc7500dcffee5459caeb031155ec 100755 (executable)
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1436,4 +1436,35 @@ test_expect_success 'recursive clone respects -q' '
        test_must_be_empty actual
 '
 
+test_expect_success '`submodule init` and `init.templateDir`' '
+       mkdir -p tmpl/hooks &&
+       write_script tmpl/hooks/post-checkout <<-EOF &&
+       echo HOOK-RUN >&2
+       echo I was here >hook.run
+       exit 1
+       EOF
+
+       test_config init.templateDir "$(pwd)/tmpl" &&
+       test_when_finished \
+               "git config --global --unset init.templateDir || true" &&
+       (
+               sane_unset GIT_TEMPLATE_DIR &&
+               NO_SET_GIT_TEMPLATE_DIR=t &&
+               export NO_SET_GIT_TEMPLATE_DIR &&
+
+               git config --global init.templateDir "$(pwd)/tmpl" &&
+               test_must_fail git submodule \
+                       add "$submodurl" sub-global 2>err &&
+               git config --global --unset init.templateDir &&
+               grep HOOK-RUN err &&
+               test_path_is_file sub-global/hook.run &&
+
+               git config init.templateDir "$(pwd)/tmpl" &&
+               git submodule add "$submodurl" sub-local 2>err &&
+               git config --unset init.templateDir &&
+               ! grep HOOK-RUN err &&
+               test_path_is_missing sub-local/hook.run
+       )
+'
+
 test_done