Re: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Richard Hansen <rhansen@xxxxxxx> writes:

> Both bash and zsh subject the value of PS1 to parameter expansion,
> command substitution, and arithmetic expansion.  Rather than include
> the raw, unescaped branch name in PS1 when running in two- or
> three-argument mode, construct PS1 to reference a variable that holds
> the branch name.  Because the shells do not recursively expand, this
> avoids arbitrary code execution by specially-crafted branch names such
> as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.
>
> Signed-off-by: Richard Hansen <rhansen@xxxxxxx>

I'd like to see this patch eyeballed by those who have been involved
in the script (shortlog and blame tells me they are SZEDER and
Simon, CC'ed), so that we can hopefully merge it by the time -rc1 is
tagged.

Will queue so that I won't lose it in the meantime.

Thanks.

>  contrib/completion/git-prompt.sh | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 7b732d2..bd7ff29 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -207,7 +207,18 @@ __git_ps1_show_upstream ()
>  			p=" u+${count#*	}-${count%	*}" ;;
>  		esac
>  		if [[ -n "$count" && -n "$name" ]]; then
> -			p="$p $(git rev-parse --abbrev-ref "$upstream" 2>/dev/null)"
> +			__git_ps1_upstream_name=$(git rev-parse \
> +				--abbrev-ref "$upstream" 2>/dev/null)
> +			if [ $pcmode = yes ]; then
> +				# see the comments around the
> +				# __git_ps1_branch_name variable below
> +				p="$p \${__git_ps1_upstream_name}"
> +			else
> +				p="$p ${__git_ps1_upstream_name}"
> +				# not needed anymore; keep user's
> +				# environment clean
> +				unset __git_ps1_upstream_name
> +			fi
>  		fi
>  	fi
>  
> @@ -438,8 +449,27 @@ __git_ps1 ()
>  		__git_ps1_colorize_gitstring
>  	fi
>  
> +	b=${b##refs/heads/}
> +	if [ $pcmode = yes ]; then
> +		# In pcmode (and only pcmode) the contents of
> +		# $gitstring are subject to expansion by the shell.
> +		# Avoid putting the raw ref name in the prompt to
> +		# protect the user from arbitrary code execution via
> +		# specially crafted ref names (e.g., a ref named
> +		# '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)' would execute
> +		# 'sudo rm -rf /' when the prompt is drawn).  Instead,
> +		# put the ref name in a new global variable (in the
> +		# __git_ps1_* namespace to avoid colliding with the
> +		# user's environment) and reference that variable from
> +		# PS1.
> +		__git_ps1_branch_name=$b
> +		# note that the $ is escaped -- the variable will be
> +		# expanded later (when it's time to draw the prompt)
> +		b="\${__git_ps1_branch_name}"
> +	fi
> +
>  	local f="$w$i$s$u"
> -	local gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"
> +	local gitstring="$c$b${f:+$z$f}$r$p"
>  
>  	if [ $pcmode = yes ]; then
>  		if [ "${__git_printf_supports_v-}" != yes ]; then
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]