[pacman-dev] [PATCH v3] makepkg: do not eval dlcmd

[Allan McRae]

On 08/09/14 02:57, lolilolicon wrote:
> This eval enables the following in a PKGBUILD to "just work":
> 
>   source=('$pkgname-$pkgver.tar.gz'::'https://host/$pkgver.tar.gz')
> 
> This has at least two problems:
> 
> - It violated the principle of least surprise.
> - It could be a security issue since URLs are arbitrary input.
> 
> Instead, expand the dlagent command line into an array, replace the %o,
> %u place holders, and run the resultant command line as is.
> 
> Embedded spaces in the DLAGENTS entry can be escaped with a backslash.
> 
> Fixes FS#41682
> ---

This is broken with bash<4.3.  There are extra quotes everywhere in the
command.

bash-4.2
+ /usr/bin/curl -fLC - --retry 3 --retry-delay 3 -o '"64.tar.xz.part"'
'"http://allanmcrae.com/tmp/64.tar.xz"'

bash-4.3
+ /usr/bin/curl -fLC - --retry 3 --retry-delay 3 -o 64.tar.xz.part
http://allanmcrae.com/tmp/64.tar.xz

@Dave: any suggestions on how to fix.


>  scripts/makepkg.sh.in | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 8e8a64c..446e9b0 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -342,8 +342,9 @@ download_file() {
>  	local proto=$(get_protocol "$netfile")
>  
>  	# find the client we should use for this URL
> -	local dlcmd
> -	dlcmd=$(get_downloadclient "$proto") || exit $?
> +	local -a cmdline
> +	IFS=' ' read -a cmdline < <(get_downloadclient "$proto")
> +	(( ${#cmdline[@]} )) || exit
>  
>  	local filename=$(get_filename "$netfile")
>  	local url=$(get_url "$netfile")
> @@ -359,20 +360,18 @@ download_file() {
>  	local dlfile="${url##*/}"
>  
>  	# replace %o by the temporary dlfile if it exists
> -	if [[ $dlcmd = *%o* ]]; then
> -		dlcmd=${dlcmd//\%o/\"$filename.part\"}
> -		dlfile="$filename.part"
> +	if [[ ${cmdline[*]} = *%o* ]]; then
> +		dlfile=$filename.part
> +		cmdline=("${cmdline[@]//%o/"$dlfile"}")
>  	fi
>  	# add the URL, either in place of %u or at the end
> -	if [[ $dlcmd = *%u* ]]; then
> -		dlcmd=${dlcmd//\%u/\"$url\"}
> +	if [[ ${cmdline[*]} = *%u* ]]; then
> +		cmdline=("${cmdline[@]//%u/"$url"}")
>  	else
> -		dlcmd="$dlcmd \"$url\""
> +		cmdline+=("$url")
>  	fi
>  
> -	local ret=0
> -	eval "$dlcmd >&2 || ret=\$?"
> -	if (( ret )); then
> +	if ! command -- "${cmdline[@]}" >&2; then
>  		[[ ! -s $dlfile ]] && rm -f -- "$dlfile"
>  		error "$(gettext "Failure while downloading %s")" "$filename"
>  		plain "$(gettext "Aborting...")"
>