| Message ID | 20190428025417.23461-1-eschwartz@archlinux.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [pacman-dev] scripts: protect against unintended glob matching in [[ ]] RHS | expand |
On 28/4/19 12:54 pm, Eli Schwartz wrote: > The right-hand side of the [[ ... = ... ]] keyword is an exception to > the general rule that quoting is unnecessary with [[ > > This is usually not a problem, e.g. in libmakepkg, lint_one_pkgname will > already fail if pkgname has an asterisk, but it certainly doesn't hurt > to be "more proper" and go with the spec; it is more dangerous in > repo-add, which can get caught in an infinite loop instead of safely > asserting there is no package named 'foo*'. > > Reported-by: Rafael Ascensão <rafa.almas@gmail.com> > Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> > --- > scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in | 2 +- > scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in | 2 +- > scripts/libmakepkg/lint_pkgbuild/depends.sh.in | 2 +- > scripts/libmakepkg/lint_pkgbuild/makedepends.sh.in | 2 +- > scripts/libmakepkg/lint_pkgbuild/optdepends.sh.in | 2 +- > scripts/libmakepkg/lint_pkgbuild/provides.sh.in | 2 +- > scripts/libmakepkg/source/git.sh.in | 2 +- > scripts/libmakepkg/tidy/zipman.sh.in | 2 +- > scripts/pacman-db-upgrade.sh.in | 2 +- > scripts/repo-add.sh.in | 4 ++-- > 10 files changed, 11 insertions(+), 11 deletions(-) Looks good. (I'll allow the formatting fix that slipped in at the end there!) A
diff --git a/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in b/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in index 0a9ddf67..df754d7e 100644 --- a/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in @@ -44,7 +44,7 @@ lint_checkdepends() { for checkdepend in "${checkdepends_list[@]}"; do name=${checkdepend%%@(<|>|=|>=|<=)*} lint_one_pkgname checkdepends "$name" || ret=1 - if [[ $name != $checkdepend ]]; then + if [[ $name != "$checkdepend" ]]; then ver=${checkdepend##$name@(<|>|=|>=|<=)} check_fullpkgver "$ver" checkdepends || ret=1 fi diff --git a/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in b/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in index b61459e1..ee0e6f50 100644 --- a/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in @@ -44,7 +44,7 @@ lint_conflicts() { for conflict in "${conflicts_list[@]}"; do name=${conflict%%@(<|>|=|>=|<=)*} lint_one_pkgname conflicts "$name" || ret=1 - if [[ $name != $conflict ]]; then + if [[ $name != "$conflict" ]]; then ver=${conflict##$name@(<|>|=|>=|<=)} check_fullpkgver "$ver" conflicts || ret=1 fi diff --git a/scripts/libmakepkg/lint_pkgbuild/depends.sh.in b/scripts/libmakepkg/lint_pkgbuild/depends.sh.in index aba43825..3fe9614f 100644 --- a/scripts/libmakepkg/lint_pkgbuild/depends.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/depends.sh.in @@ -44,7 +44,7 @@ lint_depends() { for depend in "${depends_list[@]}"; do name=${depend%%@(<|>|=|>=|<=)*} lint_one_pkgname depends "$name" || ret=1 - if [[ $name != $depend ]]; then + if [[ $name != "$depend" ]]; then ver=${depend##$name@(<|>|=|>=|<=)} # Don't validate empty version because of https://bugs.archlinux.org/task/58776 if [[ -n $ver ]]; then diff --git a/scripts/libmakepkg/lint_pkgbuild/makedepends.sh.in b/scripts/libmakepkg/lint_pkgbuild/makedepends.sh.in index 20c7f7dc..ed1c1120 100644 --- a/scripts/libmakepkg/lint_pkgbuild/makedepends.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/makedepends.sh.in @@ -44,7 +44,7 @@ lint_makedepends() { for makedepend in "${makedepends_list[@]}"; do name=${makedepend%%@(<|>|=|>=|<=)*} lint_one_pkgname makedepends "$name" || ret=1 - if [[ $name != $makedepend ]]; then + if [[ $name != "$makedepend" ]]; then ver=${makedepend##$name@(<|>|=|>=|<=)} check_fullpkgver "$ver" makedepends || ret=1 fi diff --git a/scripts/libmakepkg/lint_pkgbuild/optdepends.sh.in b/scripts/libmakepkg/lint_pkgbuild/optdepends.sh.in index 505ee848..ef7078d1 100644 --- a/scripts/libmakepkg/lint_pkgbuild/optdepends.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/optdepends.sh.in @@ -44,7 +44,7 @@ lint_optdepends() { for optdepend in "${optdepends_list[@]%%:[[:space:]]*}"; do name=${optdepend%%@(<|>|=|>=|<=)*} lint_one_pkgname optdepends "$name" || ret=1 - if [[ $name != $optdepend ]]; then + if [[ $name != "$optdepend" ]]; then ver=${optdepend##$name@(<|>|=|>=|<=)} check_fullpkgver "$ver" optdepends || ret=1 fi diff --git a/scripts/libmakepkg/lint_pkgbuild/provides.sh.in b/scripts/libmakepkg/lint_pkgbuild/provides.sh.in index 5a529728..41b4c6b9 100644 --- a/scripts/libmakepkg/lint_pkgbuild/provides.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/provides.sh.in @@ -49,7 +49,7 @@ lint_provides() { fi name=${provide%=*} lint_one_pkgname provides "$name" || ret=1 - if [[ $name != $provide ]]; then + if [[ $name != "$provide" ]]; then ver=${provide##$name=} check_fullpkgver "$ver" provides || ret=1 fi diff --git a/scripts/libmakepkg/source/git.sh.in b/scripts/libmakepkg/source/git.sh.in index 96d79623..ccf4642b 100644 --- a/scripts/libmakepkg/source/git.sh.in +++ b/scripts/libmakepkg/source/git.sh.in @@ -117,7 +117,7 @@ extract_git() { if [[ ${fragment%%=*} = tag ]]; then tagname="$(git tag -l --format='%(tag)' "$ref")" - if [[ -n $tagname && $tagname != $ref ]]; then + if [[ -n $tagname && $tagname != "$ref" ]]; then error "$(gettext "Failure while checking out version %s, the git tag has been forged")" "$ref" plain "$(gettext "Aborting...")" exit 1 diff --git a/scripts/libmakepkg/tidy/zipman.sh.in b/scripts/libmakepkg/tidy/zipman.sh.in index 43f66730..772a2ea2 100644 --- a/scripts/libmakepkg/tidy/zipman.sh.in +++ b/scripts/libmakepkg/tidy/zipman.sh.in @@ -40,7 +40,7 @@ tidy_zipman() { while read -r link ; do if [[ "${file}" -ef "${link}" ]] ; then rm -f "$link" "${link}.gz" - if [[ ${file%/*} = ${link%/*} ]]; then + if [[ ${file%/*} = "${link%/*}" ]]; then ln -s -- "${file##*/}.gz" "${link}.gz" else ln -s -- "/${file}.gz" "${link}.gz" diff --git a/scripts/pacman-db-upgrade.sh.in b/scripts/pacman-db-upgrade.sh.in index a831f181..75a108bc 100644 --- a/scripts/pacman-db-upgrade.sh.in +++ b/scripts/pacman-db-upgrade.sh.in @@ -182,7 +182,7 @@ if [[ -z "$db_version" ]]; then realdir="$(resolve_dir "$dir")" # verify realdir is inside root - if [[ ${realdir:0:${#pacroot}} != $pacroot ]]; then + if [[ ${realdir:0:${#pacroot}} != "$pacroot" ]]; then warning "$(gettext "symlink '%s' points outside pacman root, manual repair required")" "$dir" continue fi diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 57413df5..e23f47c9 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -121,7 +121,7 @@ find_pkgentry() { for pkgentry in "$tmpdir/db/$pkgname"*; do name=${pkgentry##*/} - if [[ ${name%-*-*} = $pkgname ]]; then + if [[ ${name%-*-*} = "$pkgname" ]]; then echo $pkgentry return 0 fi @@ -464,7 +464,7 @@ remove() { error "$(gettext "Package matching '%s' not found.")" "$pkgname" return 1 fi - + return 0 }
The right-hand side of the [[ ... = ... ]] keyword is an exception to the general rule that quoting is unnecessary with [[ This is usually not a problem, e.g. in libmakepkg, lint_one_pkgname will already fail if pkgname has an asterisk, but it certainly doesn't hurt to be "more proper" and go with the spec; it is more dangerous in repo-add, which can get caught in an infinite loop instead of safely asserting there is no package named 'foo*'. Reported-by: Rafael Ascensão <rafa.almas@gmail.com> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in | 2 +- scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in | 2 +- scripts/libmakepkg/lint_pkgbuild/depends.sh.in | 2 +- scripts/libmakepkg/lint_pkgbuild/makedepends.sh.in | 2 +- scripts/libmakepkg/lint_pkgbuild/optdepends.sh.in | 2 +- scripts/libmakepkg/lint_pkgbuild/provides.sh.in | 2 +- scripts/libmakepkg/source/git.sh.in | 2 +- scripts/libmakepkg/tidy/zipman.sh.in | 2 +- scripts/pacman-db-upgrade.sh.in | 2 +- scripts/repo-add.sh.in | 4 ++-- 10 files changed, 11 insertions(+), 11 deletions(-)