On Sat, Sep 06, 2014 at 04:42:08AM +0800, lolilolicon wrote:
> Something like `source=('$pkgname-$pkgver.tar.gz')` in the PKGBUILD
> would "just work" with this eval. This could be a security issue since
> URLs are arbitrary input. This also violated the principle of least
> surprise.
I still say this is security theater. The point about "least surprise"
is more convincing.
> 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 DLAGENT entry can be escaped with a backslash.
> ---
> 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..2b05468 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
The exit status of read isn't something you want to rely on here -- it
can return non-zero on EOF. Please just rely on the cmdline array being
populated or not to determine success.
Looks fine otherwise.
> 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...")"
> --
> 2.1.0
>
