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

[lolilolicon]

On Sat, Sep 6, 2014 at 3:35 AM, Dave Reisner <d at falconindy.com> wrote:
> On Sat, Sep 06, 2014 at 01:30:54AM +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 external data.
>
> Sounds somewhat contrived. External data, yes, but then you might
> consider that the *entire* PKGBUILD is external data which is fed to
> makepkg. What makes this particular case special? Is there an immediate
> problem that this fixes?
>

As I reported in bug #41682, it is surprising to have the URL eval'ed so
unexpectedly. Ask a good PKGBUILD writer, what does he expect from that
PKGBUILD?

>> -                     local agent="${i##*::}"
>> +                     local agent="${i#*::}"
>
> While I agree with this change (and other similar fixups), it seems
> unrelated to the rest of the commit...

I removed this in v2.
My bad habit of casually fixing random stuff "in scope"...

>> +     local IFS=' '
>
> How sure are you that this won't affect some later operation in the
> function?

I'll bet you a beer and a candy bar that a bad URL will break shit
sooner than this will happen.

>
>> +     cmdline=($(get_downloadclient "$proto")) || exit
>
> Why not use read to split the result?
>
>   IFS=' ' read -a cmdline < <(get_downloadclient "$proto")
>   [[ $cmdline ]] || exit

Good point. Adopted.