Re: [PATCH] git-sh-setup.sh: fix missing double quotes variables
flat view
From: Junio C Hamano <hidden>
Date: 2016-06-19 01:43:41
LE Manh Cuong [off-list ref] writes:
Leaving shell variables un-quotes can lead to security vulnerabilities. In:
: ${x=.}
`$x` is always expanded, cause `glob+split` on its result. There're some
globs is too expensive to expand, like:
x='/*/*/*/*/../../../../*/*/*/*/../../../../*/*/*/*' sh -c ':
${x=.}'
Run it and our machine will hang/crash (especially in Linux).
`LESS`, `LV` and `GIT_OBJECT_DIRECTORY` variables in `git-sh-setup` are
vulnerable with this case.
Fix this vulnerability by quoting those variables.
Signed-off-by: LE Manh Cuong <redacted>
---That is "interesting". Given that LESS, LV and GIT_OBJECT_DIRECTORY are expected to be free of any "expensive to expand" strings, I am not sure if this actually matters, though. And more importantly, these are what the end users are expected to set to whatever sensible values for them. You would not be lying if you said that Git lets people who want to do strange things shoot their feet off; I do not think that hardly deserves to be called "vulnerability", though. Having said all that, I do not mind preventing people from shooting their foot off, and the change in this patch certainly would not hurt. Thanks.
quoted hunk
git-sh-setup.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)diff --git a/git-sh-setup.sh b/git-sh-setup.sh index c48139a..85db5f1 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh@@ -160,8 +160,8 @@ git_pager() { else GIT_PAGER=cat fi - : ${LESS=-FRX} - : ${LV=-c} + : "${LESS=-FRX}" + : "${LV=-c}" export LESS LV eval "$GIT_PAGER" '"$@"'@@ -344,7 +344,7 @@ git_dir_init () { echo >&2 "Unable to determine absolute path of git directory" exit 1 } - : ${GIT_OBJECT_DIRECTORY="$(git rev-parse --git-path objects)"} + : "${GIT_OBJECT_DIRECTORY="$(git rev-parse --git-path objects)"}" } if test -z "$NONGIT_OK"