[arch-projects] [devtools] [PATCH 2/2] avoid injecting code into the format string

Eric Bélanger snowmaniscool at gmail.com
Tue Jul 30 14:51:30 EDT 2013


On Tue, Jul 30, 2013 at 11:00 AM, Dave Reisner <dreisner at archlinux.org>wrote:

> Now that die() properly forwards arguments to error(), we can expect
> that the first arg is a format string and not the entirety of the
> output.
>
> Signed-off-by: Dave Reisner <dreisner at archlinux.org>
> ---
>  arch-nspawn.in   |  4 ++--
>  archco.in        |  2 +-
>  archrelease.in   |  6 +++---
>  checkpkg.in      |  4 ++--
>  commitpkg.in     | 10 +++++-----
>  crossrepomove.in |  2 +-
>  find-libdeps.in  |  2 +-
>  makechrootpkg.in | 14 +++++++-------
>  mkarchroot.in    |  4 ++--
>  9 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/arch-nspawn.in b/arch-nspawn.in
> index 8ef39ed..f52afe9 100644
> --- a/arch-nspawn.in
> +++ b/arch-nspawn.in
> @@ -88,9 +88,9 @@ umask 0022
>
>  # Sanity check
>  if [[ ! -f "$working_dir/.arch-chroot" ]]; then
> -       die "'$working_dir' does not appear to be a Arch chroot."
> +       die "'%s' does not appear to be a Arch chroot." "$working_dir"
>  elif [[ $(cat "$working_dir/.arch-chroot") != $CHROOT_VERSION ]]; then
> -       die "chroot '$working_dir' is not at version $CHROOT_VERSION.
> Please rebuild."
> +       die "chroot '%s' is not at version %s. Please rebuild."
> "$working_dir" "$CHROOT_VERSION"
>  fi
>
>  build_mount_args
> diff --git a/archco.in b/archco.in
> index 1ee977e..29e4cd6 100644
> --- a/archco.in
> +++ b/archco.in
> @@ -15,7 +15,7 @@ case $scriptname in
>         communityco)
>                 SVNURL="svn+ssh://
> svn-community at nymeria.archlinux.org/srv/repos/svn-community/svn";;
>         *)
> -               die "Couldn't find svn url for $scriptname"
> +               die "Couldn't find svn url for %s" $scriptname"
>                 ;;
>

You forgot a quote: $scriptname" -> "$scriptname"

Eric



>  esac
>
> diff --git a/archrelease.in b/archrelease.in
> index 2e742c2..6f52dbc 100644
> --- a/archrelease.in
> +++ b/archrelease.in
> @@ -8,8 +8,8 @@ FORCE=
>  while getopts ':f' flag; do
>         case $flag in
>                 f) FORCE=1 ;;
> -               :) die "Option requires an argument -- '$OPTARG'" ;;
> -               \?) die "Invalid option -- '$OPTARG'" ;;
> +               :) die "Option requires an argument -- '%s'" "$OPTARG" ;;
> +               \?) die "Invalid option -- '%s'" "$OPTARG" ;;
>         esac
>  done
>  shift $(( OPTIND - 1 ))
> @@ -23,7 +23,7 @@ fi
>  if [[ -z $FORCE ]]; then
>         for tag in "$@"; do
>                 if ! in_array "$tag" "${_tags[@]}"; then
> -                       die 'archrelease: Invalid tag: "'$tag'" (use -f to
> force release)'
> +                       die "archrelease: Invalid tag: '%s' (use -f to
> force release)" "$tag"
>                 fi
>         done
>  fi
> diff --git a/checkpkg.in b/checkpkg.in
> index 95bf049..8e0f574 100644
> --- a/checkpkg.in
> +++ b/checkpkg.in
> @@ -41,13 +41,13 @@ for _pkgname in "${pkgname[@]}"; do
>         pkgurl=$(pacman -Spdd --print-format '%l' --noconfirm "$_pkgname")
>
>         if [[ $? -ne 0 ]]; then
> -               die "Couldn't download previous package for $_pkgname."
> +               die "Couldn't download previous package for %s."
> "$_pkgname"
>         fi
>
>         oldpkg=${pkgurl##*://*/}
>
>         if [[ ${oldpkg##*/} = ${pkgfile##*/} ]]; then
> -               die "The built package ($_pkgname) is the one in the repo
> right now!"
> +               die "The built package (%s) is the one in the repo right
> now!" "$_pkgname"
>         fi
>
>         if [[ ! -f $oldpkg ]]; then
> diff --git a/commitpkg.in b/commitpkg.in
> index db78517..ad1005b 100644
> --- a/commitpkg.in
> +++ b/commitpkg.in
> @@ -58,7 +58,7 @@ esac
>  # check if all local source files are under version control
>  for s in "${source[@]}"; do
>         if [[ $s != *://* ]] && ! svn status -v "$s@" | grep -q '^[
> AMRX~]'; then
> -               die "$s is not under version control"
> +               die "%s is not under version control" "$s"
>         fi
>  done
>
> @@ -68,7 +68,7 @@ for i in 'changelog' 'install'; do
>                 # evaluate any bash variables used
>                 eval file=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/' <<<
> "$file")\"
>                 if ! svn status -v "${file}" | grep -q '^[ AMRX~]'; then
> -                       die "${file} is not under version control"
> +                       die "%s is not under version control" "$file"
>                 fi
>         done < <(sed -n "s/^[[:space:]]*$i=//p" PKGBUILD)
>  done
> @@ -81,8 +81,8 @@ while getopts ':l:a:s:f' flag; do
>                 s) server=$OPTARG ;;
>                 l) rsyncopts+=("--bwlimit=$OPTARG") ;;
>                 a) commit_arch=$OPTARG ;;
> -               :) die "Option requires an argument -- '$OPTARG'" ;;
> -               \?) die "Invalid option -- '$OPTARG'" ;;
> +               :) die "Option requires an argument -- '%s'" "$OPTARG" ;;
> +               \?) die "Invalid option -- '%s'" "$OPTARG" ;;
>         esac
>  done
>  shift $(( OPTIND - 1 ))
> @@ -164,7 +164,7 @@ for _arch in ${arch[@]}; do
>                         gpg --detach-sign --use-agent ${SIGNWITHKEY}
> "${pkgfile}" || die
>                 fi
>                 if ! gpg --verify "$sigfile" >/dev/null 2>&1; then
> -                       die "Signature ${pkgfile}.sig is incorrect!"
> +                       die "Signature %s.sig is incorrect!" "$pkgfile"
>                 fi
>                 uploads+=("$sigfile")
>         done
> diff --git a/crossrepomove.in b/crossrepomove.in
> index 1d4ae6c..912504f 100644
> --- a/crossrepomove.in
> +++ b/crossrepomove.in
> @@ -25,7 +25,7 @@ case $scriptname in
>                 target_repo='extra'
>                 ;;
>         *)
> -               die "Couldn't find configuration for $scriptname"
> +               die "Couldn't find configuration for %s" "$scriptname"
>                 ;;
>  esac
>
> diff --git a/find-libdeps.in b/find-libdeps.in
> index 36e2c43..c9b451e 100644
> --- a/find-libdeps.in
> +++ b/find-libdeps.in
> @@ -16,7 +16,7 @@ script_mode=${0##*/find-lib}
>
>  case $script_mode in
>         deps|provides) true;;
> -       *) die "Unknown mode $script_mode" ;;
> +       *) die "Unknown mode %s" "$script_mode" ;;
>  esac
>
>  if [[ -z $1 ]]; then
> diff --git a/makechrootpkg.in b/makechrootpkg.in
> index d7d3ecf..1cd08fb 100644
> --- a/makechrootpkg.in
> +++ b/makechrootpkg.in
> @@ -81,8 +81,8 @@ done
>
>  # Canonicalize chrootdir, getting rid of trailing /
>  chrootdir=$(readlink -e "$passeddir")
> -[[ ! -d $chrootdir ]] && die "No chroot dir defined, or invalid path
> '$passeddir'"
> -[[ ! -d $chrootdir/root ]] && die "Missing chroot dir root directory. Try
> using: mkarchroot $chrootdir/root base-devel"
> +[[ ! -d $chrootdir ]] && die "No chroot dir defined, or invalid path
> '%s'" "$passeddir"
> +[[ ! -d $chrootdir/root ]] && die "Missing chroot dir root directory. Try
> using: mkarchroot %s/root base-devel" "$chrootdir"
>
>  # Detect chrootdir filesystem type
>  chroottype=$(stat -f -c %T "$chrootdir")
> @@ -136,10 +136,10 @@ create_chroot() {
>                 if [[ "$chroottype" == btrfs ]]; then
>                         if [[ -d $copydir ]]; then
>                                 btrfs subvolume delete "$copydir"
> >/dev/null ||
> -                                       die "Unable to delete subvolume
> $copydir"
> +                                       die "Unable to delete subvolume
> %s" "$copydir"
>                         fi
>                         btrfs subvolume snapshot "$chrootdir/root"
> "$copydir" >/dev/null ||
> -                               die "Unable to create subvolume $copydir"
> +                               die "Unable to create subvolume %s"
> "$copydir"
>                 else
>                         mkdir -p "$copydir"
>                         rsync -a --delete -q -W -x "$chrootdir/root/"
> "$copydir"
> @@ -155,11 +155,11 @@ clean_temporary() {
>         stat_busy "Removing temporary copy [$copy]"
>         if [[ "$chroottype" == btrfs ]]; then
>                 btrfs subvolume delete "$copydir" >/dev/null ||
> -                       die "Unable to delete subvolume $copydir"
> +                       die "Unable to delete subvolume %s" "$copydir"
>         else
>                 # avoid change of filesystem in case of an umount failure
>                 rm --recursive --force --one-file-system "$copydir" ||
> -                       die "Unable to delete $copydir"
> +                       die "Unable to delete %s" "$copydir"
>         fi
>
>         # remove lock file
> @@ -362,7 +362,7 @@ if (( ret != 0 )); then
>         if $temp_chroot; then
>                 die "Build failed"
>         else
> -               die "Build failed, check $copydir/build"
> +               die "Build failed, check %s/build" "$copydir"
>         fi
>  else
>         true
> diff --git a/mkarchroot.in b/mkarchroot.in
> index 970bbb9..7cdb274 100644
> --- a/mkarchroot.in
> +++ b/mkarchroot.in
> @@ -51,7 +51,7 @@ fi
>
>  umask 0022
>
> -[[ -e $working_dir ]] && die "Working directory '$working_dir' already
> exists"
> +[[ -e $working_dir ]] && die "Working directory '%s' already exists"
> "$working_dir"
>
>  mkdir -p "$working_dir"
>
> @@ -60,7 +60,7 @@ lock 9 "${working_dir}.lock" "Locking chroot"
>  if [[ $(stat -f -c %T "$working_dir") == btrfs ]]; then
>         rmdir "$working_dir"
>         if ! btrfs subvolume create "$working_dir"; then
> -               die "Couldn't create subvolume for '$working_dir'"
> +               die "Couldn't create subvolume for '%s'" "$working_dir"
>         fi
>         chmod 0755 "$working_dir"
>  fi
> --
> 1.8.3.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.archlinux.org/pipermail/arch-projects/attachments/20130730/c15e4794/attachment-0001.html>


More information about the arch-projects mailing list