[pacman-dev] [PATCH 1/2] lib/dload: prevent possible NULL dereference
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- Our (my) download code isn't very clear, but there's an oddball chance this might happen. Pointed out by Lucas. lib/libalpm/dload.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 5464740..3798937 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -396,7 +396,7 @@ cleanup: } } - if((ret == -1 || dload_interrupted) && should_unlink) { + if((ret == -1 || dload_interrupted) && should_unlink && tempfile) { unlink(tempfile); } -- 1.7.6
This is a safety measure to prevent simple code injection. $ i="foo bar" $ eval i="$i" bash: bar: command not found $ eval i='$i' $ echo "|$i|" |foo bar| Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- scripts/makepkg.sh.in | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index c6b522d..60f97fd 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -250,7 +250,7 @@ get_full_version() { for i in pkgver pkgrel epoch; do local indirect="${i}_override" eval $(declare -f package_$1 | sed -n "s/\(^[[:space:]]*$i=\)/${i}_override=/p") - [[ -z ${!indirect} ]] && eval "${indirect}=\${${i}}" + [[ -z ${!indirect} ]] && eval ${indirect}='${!i}' done if (( ! $epoch_override )); then echo $pkgver_override-$pkgrel_override @@ -1358,7 +1358,7 @@ create_srcpackage() { local file for file in $filelist; do # evaluate any bash variables used - eval file=${file} + eval file='${file}' if [[ ! -f "${srclinks}/${pkgbase}/$file" ]]; then msg2 "$(gettext "Adding %s file (%s)...")" "$i" "${file}" ln -s "${startdir}/$file" "${srclinks}/${pkgbase}/" @@ -1451,7 +1451,7 @@ check_sanity() { awk -F'=' '/^[[:space:]]*pkgver=/ { $1=""; print $0 }' "$BUILDFILE" | while read i _; do - eval i="$i" + eval i='$i' if [[ $i =~ [[:space:]:-] ]]; then error "$(gettext "%s is not allowed to contain colons, hyphens or whitespace.")" "pkgver" return 1 @@ -1460,7 +1460,7 @@ check_sanity() { awk -F'=' '/^[[:space:]]*pkgrel=/ { $1=""; print $0 }' "$BUILDFILE" | while read i _; do - eval i="$i" + eval i='$i' if [[ $i =~ [[:space:]-] ]]; then error "$(gettext "%s is not allowed to contain hyphens or whitespace.")" "pkgrel" return 1 @@ -1469,7 +1469,7 @@ check_sanity() { awk -F'=' '/^[[:space:]]*epoch=/ { $1=""; print $0 }' "$BUILDFILE" | while read i _; do - eval i="$i" + eval i='$i' if [[ ! $i =~ ^[0-9]*$ ]]; then error "$(gettext "%s must be an integer.")" "epoch" return 1 @@ -1538,7 +1538,7 @@ check_sanity() { local file for file in $filelist; do # evaluate any bash variables used - eval file=${file} + eval file='${file}' if [[ ! -f $file ]]; then error "$(gettext "%s file (%s) does not exist.")" "$i" "$file" ret=1 -- 1.7.6
On 19/08/11 03:57, Dave Reisner wrote:
This is a safety measure to prevent simple code injection.
$ i="foo bar" $ eval i="$i" bash: bar: command not found $ eval i='$i' $ echo "|$i|" |foo bar|
Signed-off-by: Dave Reisner<dreisner@archlinux.org>
No signoff... with single quotes it does not actually do the variable substitutions which is the whole point.
_ver=1.8.2 i='${_ver/[a-z]/.${_ver//[0-9.]/}}' echo $i ${_ver/[a-z]/.${_ver//[0-9.]/}}
eval i='$i' echo $i ${_ver/[a-z]/.${_ver//[0-9.]/}}
eval i="$i" echo $i 1.8.2
So what is really needed is: eval i=\"$i\" Allan
On Fri, Aug 19, 2011 at 05:47:48AM +1000, Allan McRae wrote:
On 19/08/11 03:57, Dave Reisner wrote:
This is a safety measure to prevent simple code injection.
$ i="foo bar" $ eval i="$i" bash: bar: command not found $ eval i='$i' $ echo "|$i|" |foo bar|
Signed-off-by: Dave Reisner<dreisner@archlinux.org>
No signoff... with single quotes it does not actually do the variable substitutions which is the whole point.
_ver=1.8.2 i='${_ver/[a-z]/.${_ver//[0-9.]/}}' echo $i ${_ver/[a-z]/.${_ver//[0-9.]/}}
eval i='$i' echo $i ${_ver/[a-z]/.${_ver//[0-9.]/}}
eval i="$i" echo $i 1.8.2
So what is really needed is: eval i=\"$i\"
Allan
I hate eval. I'll fix my patch. d
On Thu, Aug 18, 2011 at 01:57:33PM -0400, Dave Reisner wrote:
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- Our (my) download code isn't very clear, but there's an oddball chance this might happen. Pointed out by Lucas.
Still a bit hackish, but definitely looks better than mine :) Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de>
lib/libalpm/dload.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 5464740..3798937 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -396,7 +396,7 @@ cleanup: } }
- if((ret == -1 || dload_interrupted) && should_unlink) { + if((ret == -1 || dload_interrupted) && should_unlink && tempfile) { unlink(tempfile); }
-- 1.7.6
participants (3)
-
Allan McRae
-
Dave Reisner
-
Lukas Fleischer