@matheusbsbs / matheus-bagliano / commits / 57ea7123c8

git.c: make sure we do not leak GIT_* to alias scripts

The unfortunate commit d95138e (setup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR - 2015-06-26) exposes another problem, besides git-clone that's described in the previous commit. If GIT_WORK_TREE (or even GIT_DIR) is exported to an alias script, it may mislead git commands in the script where the repo is. Granted, most scripts work on the repo where the alias is summoned from. But nowhere do we forbid the script to visit another repository. The revert of d95138e in the previous commit is sufficient as a fix. However, to protect us from accidentally leaking GIT_* environment variables again, we restore certain sensitive env before calling the external script. GIT_PREFIX is let through because there's another setup side effect that we simply accepted so far: current working directory is moved. Maybe in future we can introduce a new alias format that guarantees no cwd move, then we can unexport GIT_PREFIX. Reported-by: Gabriel Ganne <gabriel.ganne@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Dec 20, 2015 at 14:50 UTC 57ea7123c86771f47f34e7d92d1822d8b429897a

2 files changed +24 -3

git.c

+7 -3

@@ -41,13 +41,16 @@ static void save_env_before_alias(void)

41 }

42 }

44 -static void restore_env(void)

44 +static void restore_env(int external_alias)

45 {

46 int i;

47 - if (orig_cwd && chdir(orig_cwd))

47 + if (!external_alias && orig_cwd && chdir(orig_cwd))

48 die_errno("could not move to %s", orig_cwd);

49 free(orig_cwd);

50 for (i = 0; i < ARRAY_SIZE(env_names); i++) {

51 + if (external_alias &&

52 + !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))

53 + continue;

54 if (orig_env[i])

55 setenv(env_names[i], orig_env[i], 1);

56 else

@@ -243,6 +246,7 @@ static int handle_alias(int *argcp, const char ***argv)

246 int argc = *argcp, i;

247

248 commit_pager_choice();

249 + restore_env(1);

250

251 /* build alias_argv */

252 alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 1));

@@ -291,7 +295,7 @@ static int handle_alias(int *argcp, const char ***argv)

295 ret = 1;

296 }

297

294 - restore_env();

298 + restore_env(0);

299

300 errno = saved_errno;

301

t/t0001-init.sh

+17

@@ -87,6 +87,23 @@ test_expect_success 'plain nested in bare through aliased command' '

87 check_config bare-ancestor-aliased.git/plain-nested/.git false unset

88 '

90 +test_expect_success 'No extra GIT_* on alias scripts' '

91 + (

92 + env | sed -ne "/^GIT_/s/=.*//p" &&

93 + echo GIT_PREFIX && # setup.c

94 + echo GIT_TEXTDOMAINDIR # wrapper-for-bin.sh

95 + ) | sort | uniq >expected &&

96 + cat <<-\EOF >script &&

97 + #!/bin/sh

98 + env | sed -ne "/^GIT_/s/=.*//p" | sort >actual

99 + exit 0

100 + EOF

101 + chmod 755 script &&

102 + git config alias.script \!./script &&

103 + ( mkdir sub && cd sub && git script ) &&

104 + test_cmp expected actual

105 +'

106 +

107 test_expect_success 'plain with GIT_WORK_TREE' '

108 mkdir plain-wt &&

109 test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt