LE Manh Cuong <cuong.manhle.vn@xxxxxxxxx> 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 <cuong.manhle.vn@xxxxxxxxx>
> ---
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.
> 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"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html