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

[Andrew Gregory]

On 09/05/14 at 03:35pm, Dave Reisner 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?
> 
> > Instead, expand the dlagent command line into an array, replace the %o,
> > %u place holders, and run the resultant command line as is.
> > 
> > A shortcoming of this is the dlagent command line cannot have embedded
> > spaces. If this turns out to be a problem, we can do an early eval
> > before substituting %o and %u.
> > ---
> >  scripts/makepkg.sh.in | 30 ++++++++++++++----------------
> >  1 file changed, 14 insertions(+), 16 deletions(-)
> > 
> > diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> > index 8e8a64c..a134453 100644
> > --- a/scripts/makepkg.sh.in
> > +++ b/scripts/makepkg.sh.in
> > @@ -288,12 +288,12 @@ get_protocol() {
> >  get_downloadclient() {
> >  	local proto=$1
> >  
> > -	# loop through DOWNLOAD_AGENTS variable looking for protocol
> > +	# loop through DLAGENTS variable looking for protocol
> >  	local i
> >  	for i in "${DLAGENTS[@]}"; do
> >  		local handler="${i%%::*}"
> >  		if [[ $proto = "$handler" ]]; then
> > -			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...

There is also an existing patch that already does this:
https://patchwork.archlinux.org/patch/2150/

apg