@matheusbsbs / matheus-bagliano / commits / 940283101c

run-command: restrict PATH search to executable files

In some situations run-command will incorrectly try (and fail) to execute a directory instead of an executable file. This was observed by having a directory called "ssh" in $PATH before the real ssh and trying to use ssh protoccol, reslting in the following: $ git ls-remote ssh://url fatal: cannot exec 'ssh': Permission denied It ends up being worse and run-command will even try to execute a non-executable file if it preceeds the executable version of a file on the PATH. For example, if PATH=~/bin1:~/bin2:~/bin3 and there exists a directory 'git-hello' in 'bin1', a non-executable file 'git-hello' in bin2 and an executable file 'git-hello' (which prints "Hello World!") in bin3 the following will occur: $ git hello fatal: cannot exec 'git-hello': Permission denied This is due to only checking 'access()' when locating an executable in PATH, which doesn't distinguish between files and directories. Instead use 'is_executable()' which check that the path is to a regular, executable file. Now run-command won't try to execute the directory or non-executable file 'git-hello': $ git hello Hello World! which matches what execvp(3) would have done when asked to execute git-hello with such a $PATH. Reported-by: Brian Hatfield <bhatfield@google.com> Signed-off-by: Brandon Williams <bmwill@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Williams committed Apr 25, 2017 at 16:47 UTC 940283101ce87250cf31a592730386f5061e1286

2 files changed +48 -1

run-command.c

+18 -1

@@ -159,6 +159,23 @@ int is_executable(const char *name)

159 return st.st_mode & S_IXUSR;

160 }

161

162 +/*

163 + * Search $PATH for a command. This emulates the path search that

164 + * execvp would perform, without actually executing the command so it

165 + * can be used before fork() to prepare to run a command using

166 + * execve() or after execvp() to diagnose why it failed.

167 + *

168 + * The caller should ensure that file contains no directory

169 + * separators.

170 + *

171 + * Returns the path to the command, as found in $PATH or NULL if the

172 + * command could not be found. The caller inherits ownership of the memory

173 + * used to store the resultant path.

174 + *

175 + * This should not be used on Windows, where the $PATH search rules

176 + * are more complicated (e.g., a search for "foo" should find

177 + * "foo.exe").

178 + */

179 static char *locate_in_PATH(const char *file)

180 {

181 const char *p = getenv("PATH");

@@ -179,7 +196,7 @@ static char *locate_in_PATH(const char *file)

196 }

197 strbuf_addstr(&buf, file);

198

182 - if (!access(buf.buf, F_OK))

199 + if (is_executable(buf.buf))

200 return strbuf_detach(&buf, NULL);

201

202 if (!*end)

t/t0061-run-command.sh

+30

@@ -37,6 +37,36 @@ test_expect_success !MINGW 'run_command can run a script without a #! line' '

37 test_cmp empty err

38 '

40 +test_expect_success 'run_command does not try to execute a directory' '

41 + test_when_finished "rm -rf bin1 bin2" &&

42 + mkdir -p bin1/greet bin2 &&

43 + write_script bin2/greet <<-\EOF &&

44 + cat bin2/greet

45 + EOF

46 +

47 + PATH=$PWD/bin1:$PWD/bin2:$PATH \

48 + test-run-command run-command greet >actual 2>err &&

49 + test_cmp bin2/greet actual &&

50 + test_cmp empty err

51 +'

52 +

53 +test_expect_success POSIXPERM 'run_command passes over non-executable file' '

54 + test_when_finished "rm -rf bin1 bin2" &&

55 + mkdir -p bin1 bin2 &&

56 + write_script bin1/greet <<-\EOF &&

57 + cat bin1/greet

58 + EOF

59 + chmod -x bin1/greet &&

60 + write_script bin2/greet <<-\EOF &&

61 + cat bin2/greet

62 + EOF

63 +

64 + PATH=$PWD/bin1:$PWD/bin2:$PATH \

65 + test-run-command run-command greet >actual 2>err &&

66 + test_cmp bin2/greet actual &&

67 + test_cmp empty err

68 +'

69 +

70 test_expect_success POSIXPERM 'run_command reports EACCES' '

71 cat hello-script >hello.sh &&

72 chmod -x hello.sh &&