Browse Source

avoid evaluating shell in paths used in apt-key

apt-key creates internally a script (since ~1.1) which it will call to
avoid dealing with an array of different options in the code itself, but
while writing this script it wraps the values in "", which will cause
the shell to evaluate its content upon execution.
To make 'use' of this either set a absolute gpg command or TMPDIR to
something as interesting as:
"/tmp/This is fü\$\$ing cràzy, \$(man man | head -n1 | cut -d' ' -f1)\$!"

If such paths can be encountered in reality is a different question…
David Kalnischkies 10 years ago
parent
commit
bc8f83a5af
3 changed files with 45 additions and 23 deletions
  1. 24 20
      cmdline/apt-key.in
  2. 13 3
      test/integration/framework
  3. 8 0
      test/integration/test-apt-key

+ 24 - 20
cmdline/apt-key.in

@@ -4,17 +4,14 @@ set -e
 unset GREP_OPTIONS
 unset GREP_OPTIONS
 export IFS="$(printf "\n\b")"
 export IFS="$(printf "\n\b")"
 
 
-APT_DIR="/"
-eval $(apt-config shell APT_DIR Dir)
-
 MASTER_KEYRING='&keyring-master-filename;'
 MASTER_KEYRING='&keyring-master-filename;'
-eval $(apt-config shell MASTER_KEYRING APT::Key::MasterKeyring)
+eval "$(apt-config shell MASTER_KEYRING APT::Key::MasterKeyring)"
 ARCHIVE_KEYRING='&keyring-filename;'
 ARCHIVE_KEYRING='&keyring-filename;'
-eval $(apt-config shell ARCHIVE_KEYRING APT::Key::ArchiveKeyring)
+eval "$(apt-config shell ARCHIVE_KEYRING APT::Key::ArchiveKeyring)"
 REMOVED_KEYS='&keyring-removed-filename;'
 REMOVED_KEYS='&keyring-removed-filename;'
-eval $(apt-config shell REMOVED_KEYS APT::Key::RemovedKeys)
+eval "$(apt-config shell REMOVED_KEYS APT::Key::RemovedKeys)"
 ARCHIVE_KEYRING_URI='&keyring-uri;'
 ARCHIVE_KEYRING_URI='&keyring-uri;'
-eval $(apt-config shell ARCHIVE_KEYRING_URI APT::Key::ArchiveKeyringURI)
+eval "$(apt-config shell ARCHIVE_KEYRING_URI APT::Key::ArchiveKeyringURI)"
 
 
 aptkey_echo() { echo "$@"; }
 aptkey_echo() { echo "$@"; }
 
 
@@ -40,6 +37,10 @@ command_available() {
     return 1
     return 1
 }
 }
 
 
+escape_shell() {
+    echo "$@" | sed -e "s#'#'\"'\"'#g"
+}
+
 get_fingerprints_of_keyring() {
 get_fingerprints_of_keyring() {
     aptkey_execute "$GPG_SH" --keyring "$1" --with-colons --fingerprint | while read publine; do
     aptkey_execute "$GPG_SH" --keyring "$1" --with-colons --fingerprint | while read publine; do
 	# search for a public key
 	# search for a public key
@@ -55,8 +56,8 @@ get_fingerprints_of_keyring() {
 }
 }
 
 
 add_keys_with_verify_against_master_keyring() {
 add_keys_with_verify_against_master_keyring() {
-    ADD_KEYRING=$1
-    MASTER=$2
+    ADD_KEYRING="$1"
+    MASTER="$2"
 
 
     if [ ! -f "$ADD_KEYRING" ]; then
     if [ ! -f "$ADD_KEYRING" ]; then
 	echo >&2 "ERROR: '$ADD_KEYRING' not found"
 	echo >&2 "ERROR: '$ADD_KEYRING' not found"
@@ -72,8 +73,8 @@ add_keys_with_verify_against_master_keyring() {
     #   all keys that are exported must have a valid signature
     #   all keys that are exported must have a valid signature
     #   from a key in the $distro-master-keyring
     #   from a key in the $distro-master-keyring
     add_keys="$(get_fingerprints_of_keyring "$ADD_KEYRING")"
     add_keys="$(get_fingerprints_of_keyring "$ADD_KEYRING")"
-    all_add_keys=`aptkey_execute "$GPG_SH" --keyring "$ADD_KEYRING" --with-colons --list-keys | grep ^[ps]ub | cut -d: -f5`
-    master_keys=`aptkey_execute "$GPG_SH" --keyring "$MASTER" --with-colons --list-keys | grep ^pub | cut -d: -f5`
+    all_add_keys="$(aptkey_execute "$GPG_SH" --keyring "$ADD_KEYRING" --with-colons --list-keys | grep ^[ps]ub | cut -d: -f5)"
+    master_keys="$(aptkey_execute "$GPG_SH" --keyring "$MASTER" --with-colons --list-keys | grep ^pub | cut -d: -f5)"
 
 
     # ensure there are no colisions LP: #857472
     # ensure there are no colisions LP: #857472
     for all_add_key in $all_add_keys; do
     for all_add_key in $all_add_keys; do
@@ -114,6 +115,9 @@ add_keys_with_verify_against_master_keyring() {
 # the archive-keyring keys needs to be signed with the master key
 # the archive-keyring keys needs to be signed with the master key
 # (otherwise it does not make sense from a security POV)
 # (otherwise it does not make sense from a security POV)
 net_update() {
 net_update() {
+    local APT_DIR='/'
+    eval $(apt-config shell APT_DIR Dir)
+
     # Disabled for now as code is insecure (LP: #1013639 (and 857472, 1013128))
     # Disabled for now as code is insecure (LP: #1013639 (and 857472, 1013128))
     APT_KEY_NET_UPDATE_ENABLED=""
     APT_KEY_NET_UPDATE_ENABLED=""
     eval $(apt-config shell APT_KEY_NET_UPDATE_ENABLED APT::Key::Net-Update-Enabled)
     eval $(apt-config shell APT_KEY_NET_UPDATE_ENABLED APT::Key::Net-Update-Enabled)
@@ -137,7 +141,7 @@ net_update() {
     keyring="${APT_DIR}/var/lib/apt/keyrings/$(basename "$ARCHIVE_KEYRING_URI")"
     keyring="${APT_DIR}/var/lib/apt/keyrings/$(basename "$ARCHIVE_KEYRING_URI")"
     old_mtime=0
     old_mtime=0
     if [ -e $keyring ]; then
     if [ -e $keyring ]; then
-	old_mtime=$(stat -c %Y $keyring)
+	old_mtime=$(stat -c %Y "$keyring")
     fi
     fi
     (cd  "${APT_DIR}/var/lib/apt/keyrings"; wget --timeout=90 -q -N "$ARCHIVE_KEYRING_URI")
     (cd  "${APT_DIR}/var/lib/apt/keyrings"; wget --timeout=90 -q -N "$ARCHIVE_KEYRING_URI")
     if [ ! -e "$keyring" ]; then
     if [ ! -e "$keyring" ]; then
@@ -228,7 +232,7 @@ foreach_keyring_do() {
 	    $ACTION "$TRUSTEDFILE" "$@"
 	    $ACTION "$TRUSTEDFILE" "$@"
 	fi
 	fi
 	local TRUSTEDPARTS="/etc/apt/trusted.gpg.d"
 	local TRUSTEDPARTS="/etc/apt/trusted.gpg.d"
-	eval $(apt-config shell TRUSTEDPARTS Dir::Etc::TrustedParts/d)
+	eval "$(apt-config shell TRUSTEDPARTS Dir::Etc::TrustedParts/d)"
 	if [ -d "$TRUSTEDPARTS" ]; then
 	if [ -d "$TRUSTEDPARTS" ]; then
 	    # strip / suffix as gpg will double-slash in that case (#665411)
 	    # strip / suffix as gpg will double-slash in that case (#665411)
 	    local STRIPPED_TRUSTEDPARTS="${TRUSTEDPARTS%/}"
 	    local STRIPPED_TRUSTEDPARTS="${TRUSTEDPARTS%/}"
@@ -360,7 +364,7 @@ setup_merged_keyring() {
 	FORCED_KEYRING="${GPGHOMEDIR}/forcedkeyid.gpg"
 	FORCED_KEYRING="${GPGHOMEDIR}/forcedkeyid.gpg"
 	TRUSTEDFILE="${FORCED_KEYRING}"
 	TRUSTEDFILE="${FORCED_KEYRING}"
 	echo "#!/bin/sh
 	echo "#!/bin/sh
-exec sh \"${GPG}\" --keyring \"${TRUSTEDFILE}\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh"
+exec sh '($(escape_shell "${GPG}")' --keyring '$(escape_shell "${TRUSTEDFILE}")' \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh"
 	GPG="${GPGHOMEDIR}/gpg.1.sh"
 	GPG="${GPGHOMEDIR}/gpg.1.sh"
 	# ignore error as this "just" means we haven't found the forced keyid and the keyring will be empty
 	# ignore error as this "just" means we haven't found the forced keyid and the keyring will be empty
 	import_keyring_into_keyring '' "$TRUSTEDFILE" "$FORCED_KEYID" || true
 	import_keyring_into_keyring '' "$TRUSTEDFILE" "$FORCED_KEYID" || true
@@ -372,12 +376,12 @@ exec sh \"${GPG}\" --keyring \"${TRUSTEDFILE}\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1.
 	   touch "${GPGHOMEDIR}/pubring.gpg" "${GPGHOMEDIR}/pubring.orig.gpg"
 	   touch "${GPGHOMEDIR}/pubring.gpg" "${GPGHOMEDIR}/pubring.orig.gpg"
 	fi
 	fi
 	echo "#!/bin/sh
 	echo "#!/bin/sh
-exec sh \"${GPG}\" --keyring \"${GPGHOMEDIR}/pubring.gpg\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh"
+exec sh '$(escape_shell "${GPG}")' --keyring '$(escape_shell "${GPGHOMEDIR}/pubring.gpg")' \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh"
 	GPG="${GPGHOMEDIR}/gpg.1.sh"
 	GPG="${GPGHOMEDIR}/gpg.1.sh"
     else
     else
 	create_new_keyring "$TRUSTEDFILE"
 	create_new_keyring "$TRUSTEDFILE"
 	echo "#!/bin/sh
 	echo "#!/bin/sh
-exec sh \"${GPG}\" --keyring \"${TRUSTEDFILE}\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh"
+exec sh '$(escape_shell "${GPG}")' --keyring '$(escape_shell "${TRUSTEDFILE}")' \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh"
 	GPG="${GPGHOMEDIR}/gpg.1.sh"
 	GPG="${GPGHOMEDIR}/gpg.1.sh"
     fi
     fi
 }
 }
@@ -479,13 +483,13 @@ create_gpg_home() {
 	fi
 	fi
     fi
     fi
     GPGHOMEDIR="$(mktemp -d)"
     GPGHOMEDIR="$(mktemp -d)"
-    CURRENTTRAP="${CURRENTTRAP} rm -rf '${GPGHOMEDIR}';"
+    CURRENTTRAP="${CURRENTTRAP} rm -rf '$(escape_shell "${GPGHOMEDIR}")';"
     trap "${CURRENTTRAP}" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM
     trap "${CURRENTTRAP}" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM
     chmod 700 "$GPGHOMEDIR"
     chmod 700 "$GPGHOMEDIR"
 }
 }
 
 
 prepare_gpg_home() {
 prepare_gpg_home() {
-    eval $(apt-config shell GPG_EXE Apt::Key::gpgcommand)
+    eval "$(apt-config shell GPG_EXE Apt::Key::gpgcommand)"
 
 
     if [ -n "$GPG_EXE" ] && command_available "$GPG_EXE"; then
     if [ -n "$GPG_EXE" ] && command_available "$GPG_EXE"; then
 	true
 	true
@@ -518,8 +522,8 @@ prepare_gpg_home() {
     fi
     fi
     # tell gpg that it shouldn't try to maintain a trustdb file
     # tell gpg that it shouldn't try to maintain a trustdb file
     echo "#!/bin/sh
     echo "#!/bin/sh
-exec \"${GPG_EXE}\" --ignore-time-conflict --no-options --no-default-keyring \\
-   --homedir \"${GPGHOMEDIR}\" --no-auto-check-trustdb --trust-model always \"\$@\"" > "${GPGHOMEDIR}/gpg.0.sh"
+exec '$(escape_shell "${GPG_EXE}")' --ignore-time-conflict --no-options --no-default-keyring \\
+--homedir '$(escape_shell "${GPGHOMEDIR}")' --no-auto-check-trustdb --trust-model always \"\$@\"" > "${GPGHOMEDIR}/gpg.0.sh"
     GPG_SH="${GPGHOMEDIR}/gpg.0.sh"
     GPG_SH="${GPGHOMEDIR}/gpg.0.sh"
     GPG="$GPG_SH"
     GPG="$GPG_SH"
 
 

+ 13 - 3
test/integration/framework

@@ -54,7 +54,7 @@ msgprintf() {
 		printf "$START " "$1"
 		printf "$START " "$1"
 		shift
 		shift
 		while [ -n "$1" ]; do
 		while [ -n "$1" ]; do
-			printf "$MIDDLE " "$(echo "$1" | sed -e 's#^apt\([cfghs]\)#apt-\1#')"
+			printf "$MIDDLE " "$(echo "$1" | sed -e 's#^apt\([cfghks]\)#apt-\1#')"
 			shift
 			shift
 		done
 		done
 	fi
 	fi
@@ -245,13 +245,23 @@ addtrap() {
 	trap "shellsetedetector; $CURRENTTRAP exitwithstatus;" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM
 	trap "shellsetedetector; $CURRENTTRAP exitwithstatus;" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM
 }
 }
 
 
+escape_shell() {
+    echo "$@" | sed -e "s#'#'\"'\"'#g"
+}
+
 setupenvironment() {
 setupenvironment() {
 	# privilege dropping and testing doesn't work if /tmp isn't world-writeable (as e.g. with libpam-tmpdir)
 	# privilege dropping and testing doesn't work if /tmp isn't world-writeable (as e.g. with libpam-tmpdir)
 	if [ -n "$TMPDIR" ] && [ "$(id -u)" = '0' ] && [ "$(stat --format '%a' "$TMPDIR")" != '1777' ]; then
 	if [ -n "$TMPDIR" ] && [ "$(id -u)" = '0' ] && [ "$(stat --format '%a' "$TMPDIR")" != '1777' ]; then
 		unset TMPDIR
 		unset TMPDIR
 	fi
 	fi
 	TMPWORKINGDIRECTORY="$(mktemp -d)"
 	TMPWORKINGDIRECTORY="$(mktemp -d)"
-	addtrap "cd /; rm -rf \"$TMPWORKINGDIRECTORY\";"
+	addtrap "cd /; rm -rf '$(escape_shell "$TMPWORKINGDIRECTORY")';"
+	if [ -n "$TMPDIR_ADD" ]; then
+		TMPWORKINGDIRECTORY="${TMPWORKINGDIRECTORY}/${TMPDIR_ADD}"
+		mkdir -p "$TMPWORKINGDIRECTORY"
+		unset TMPDIR_ADD
+		export TMPDIR="$TMPWORKINGDIRECTORY"
+	fi
 	msgninfo "Preparing environment for ${0##*/} in ${TMPWORKINGDIRECTORY}…"
 	msgninfo "Preparing environment for ${0##*/} in ${TMPWORKINGDIRECTORY}…"
 
 
 	mkdir -m 700 "${TMPWORKINGDIRECTORY}/downloaded"
 	mkdir -m 700 "${TMPWORKINGDIRECTORY}/downloaded"
@@ -1254,7 +1264,7 @@ EOF
 	# start with an unmounted disk
 	# start with an unmounted disk
 	mv "${CD}" "${CD}-unmounted"
 	mv "${CD}" "${CD}-unmounted"
 	# we don't want the disk to be modifiable
 	# we don't want the disk to be modifiable
-	addtrap 'prefix' "chmod -f -R +w \"$PWD/rootdir/media/cdrom/dists/\" \"$PWD/rootdir/media/cdrom-unmounted/dists/\" || true;"
+	addtrap 'prefix' "chmod -f -R +w '$(escape_shell "$PWD/rootdir/media/cdrom/dists/")' '$(escape_shell "$PWD/rootdir/media/cdrom-unmounted/dists/")' || true;"
 	chmod -R 555 rootdir/media/cdrom-unmounted/dists
 	chmod -R 555 rootdir/media/cdrom-unmounted/dists
 }
 }
 
 

+ 8 - 0
test/integration/test-apt-key

@@ -1,6 +1,13 @@
 #!/bin/sh
 #!/bin/sh
 set -e
 set -e
 
 
+# apt-key is a shell script, so relatively prune to be effected by 'crazy' things:
+# confuses config parser as there exists no way of escaping " currently.
+#TMPDIR="$(mktemp -d)/This is \"fü\$\$ing cràzy\", \$(man man | head -n1 | cut -d' ' -f 1)\$!"
+# gpg doesn't like | in path names – documented e.g. in the man gpg2 --agent-program
+#TMPDIR="$(mktemp -d)/This is fü\$\$ing cràzy, \$(man man | head -n1 | cut -d' ' -f 1)\$!"
+TMPDIR_ADD="This is fü\$\$ing cràzy, \$(apt -v)\$!"
+
 TESTDIR="$(readlink -f "$(dirname "$0")")"
 TESTDIR="$(readlink -f "$(dirname "$0")")"
 . "$TESTDIR/framework"
 . "$TESTDIR/framework"
 
 
@@ -187,6 +194,7 @@ gpg:              unchanged: 1' aptkey --fakeroot update
 	echo 'Verify me. This is my signature.' > signature
 	echo 'Verify me. This is my signature.' > signature
 	testsuccess --nomsg aptkey --quiet --keyring keys/marvinparanoid.pub --secret-keyring keys/marvinparanoid.sec --readonly \
 	testsuccess --nomsg aptkey --quiet --keyring keys/marvinparanoid.pub --secret-keyring keys/marvinparanoid.sec --readonly \
 		adv --batch --yes --default-key 'Marvin' --armor --detach-sign --sign --output signature.gpg signature
 		adv --batch --yes --default-key 'Marvin' --armor --detach-sign --sign --output signature.gpg signature
+	testsuccess test -s signature.gpg -a -s signature
 
 
 
 
 	for GPGV in '' 'gpgv' 'gpgv2'; do
 	for GPGV in '' 'gpgv' 'gpgv2'; do