FS#43706 - [checkupdates] Avoiding using the 'eval' command

Attached to Project: Pacman
Opened by Baeyens (berbae) - Thursday, 05 February 2015, 14:13 GMT
Last edited by Allan McRae (Allan) - Tuesday, 11 October 2016, 10:34 GMT
Task Type Bug Report
Category Scripts & Tools
Status Closed
Assigned To Andrew Gregory (andrewgregory)
Architecture All
Severity Medium
Priority Normal
Reported Version 4.2.0
Due in Version 5.1.0
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

The 'eval' command is risky to use when what is eval-ed is not under the developer control.
That is the case here with this line:

eval $(awk -F' *= *' '$1 ~ /DBPath/ { print $1 "=" $2 }' /etc/pacman.conf)

If a maligned man adds a bad command at the end of a 'DBPath=' line, like:

DBPath = /var/lib/pacman/;rm ...

the 'rm ...' will be executed by the 'eval' command, which is bad.

I submit a patch to correct this, which avoids the 'eval' command and secures the script:
http://pastebin.com/raw.php?i=FeXx1vDL
This task depends upon

Closed by  Allan McRae (Allan)
Tuesday, 11 October 2016, 10:34 GMT
Reason for closing:  Won't fix
Additional comments about closing:  Removed from repo
Comment by Kyle Keen (keenerd) - Thursday, 05 February 2015, 14:28 GMT
That particular method of getting variables from the conf file is common to several utilities provided with pacman:

pacdiff
pacman-optimize
bacman

Possibly others, this was just from a casual grep.
Comment by Dave Reisner (falconindy) - Thursday, 05 February 2015, 15:23 GMT
/etc/pacman.conf is owned by root. If a malicious user or attacker has the ability to modify your /etc/pacman.conf, there's a whole host of other things they can do to your system.

I'll agree that eval is a bad, but it's really not that bad for the reason you're proposing here.
Comment by Baeyens (berbae) - Thursday, 05 February 2015, 22:03 GMT
I agree it's a little paranoid, but being more secure is better than less.
The changes to the scripts are not so great, and this is nevertheless a possible vulnerability with great consequences if exploited in a way or another.

Comment by Allan McRae (Allan) - Friday, 06 February 2015, 00:05 GMT
Can't we just use:
DBPath="$(awk -F' *= *' '/DBPath/ { print $2 }' /etc/pacman.conf)"
Comment by Baeyens (berbae) - Friday, 06 February 2015, 13:39 GMT
Allan, your proposed fix is really simpler;
followed by:
[[ -d "$DBPath" ]] || exit 1
this should be perfect and very easy to include in a future realease.
Thanks.
Comment by Dave Reisner (falconindy) - Friday, 06 February 2015, 13:44 GMT
It's simpler, but it's still not solving any real problems. If you want proper motivation to fix this, consider that:

DBPath = /path/to some/db

Is valid and handled correctly by pacman, but isn't handled correctly by checkupdates (currently) or any of the proposed fixes.
Comment by Andrew Gregory (andrewgregory) - Saturday, 07 February 2015, 06:13 GMT
I have a config parser we can use for just this: https://github.com/andrewgregory/pacutils#pacconf
Comment by Vasil Yonkov (spirtbrat) - Friday, 02 October 2015, 09:25 GMT
I believe 'eval' is used here, because 'DBPath' is a variable which has to be used inside the awk expression. There is more than one way to do that, but the one that works with all versions of awk - even the most ancient ones - is to close the single quotes just before the variable, evaluate the variable, and then open again the single quotes to continue with the awk expression.
Like that:

awk -F' *= *' '$1 ~ /'"$DBPath"'/ { print $1 "=" $2 }' /etc/pacman.conf

Loading...