From ffdcadf08464d61b839f076d39ae3b500daa1971 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Aug 2020 20:05:11 +0200 Subject: [PATCH] Fix printf escape errors in shell scripts Fix `printf "$foo"` which treats the value of `foo` as a printf format rather than a string. I used the following command to find potentially problematic lines: ``` git ls-files '*.sh' | xargs egrep 'printf +("?[^"]*|[^ ]*)\$' ``` The remaining ones are false positives for this regexp. The errors only had minor consequences: the output of `ssl-opt.sh` contained lines like ``` Renegotiation: gnutls server strict, client-initiated .................. ./tests/ssl-opt.sh: 741: printf: %S: invalid directive PASS ``` and in case of failure the GnuTLS command containing a substring like `--priority=NORMAL:%SAFE_RENEGOTIATION` was not included in the log file. With the current tests, there was no risk of a test failure going undetected. Signed-off-by: Gilles Peskine --- tests/compat.sh | 14 +++++++------- tests/scripts/check-names.sh | 4 ++-- tests/ssl-opt.sh | 24 ++++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/compat.sh b/tests/compat.sh index 7796bd2fa..8905430c7 100755 --- a/tests/compat.sh +++ b/tests/compat.sh @@ -117,12 +117,12 @@ PEERS="OpenSSL$PEER_GNUTLS mbedTLS" print_usage() { echo "Usage: $0" printf " -h|--help\tPrint this help.\n" - printf " -f|--filter\tOnly matching ciphersuites are tested (Default: '$FILTER')\n" - printf " -e|--exclude\tMatching ciphersuites are excluded (Default: '$EXCLUDE')\n" - printf " -m|--modes\tWhich modes to perform (Default: '$MODES')\n" - printf " -t|--types\tWhich key exchange type to perform (Default: '$TYPES')\n" - printf " -V|--verify\tWhich verification modes to perform (Default: '$VERIFIES')\n" - printf " -p|--peers\tWhich peers to use (Default: '$PEERS')\n" + printf " -f|--filter\tOnly matching ciphersuites are tested (Default: '%s')\n" "$FILTER" + printf " -e|--exclude\tMatching ciphersuites are excluded (Default: '%s')\n" "$EXCLUDE" + printf " -m|--modes\tWhich modes to perform (Default: '%s')\n" "$MODES" + printf " -t|--types\tWhich key exchange type to perform (Default: '%s')\n" "$TYPES" + printf " -V|--verify\tWhich verification modes to perform (Default: '%s')\n" "$VERIFIES" + printf " -p|--peers\tWhich peers to use (Default: '%s')\n" "$PEERS" printf " \tAlso available: GnuTLS (needs v3.2.15 or higher)\n" printf " -M|--memcheck\tCheck memory leaks and errors.\n" printf " -v|--verbose\tSet verbose output.\n" @@ -1134,7 +1134,7 @@ run_client() { VERIF=$(echo $VERIFY | tr '[:upper:]' '[:lower:]') TITLE="`echo $1 | head -c1`->`echo $SERVER_NAME | head -c1`" TITLE="$TITLE $MODE,$VERIF $2" - printf "$TITLE " + printf "%s " "$TITLE" LEN=$(( 72 - `echo "$TITLE" | wc -c` )) for i in `seq 1 $LEN`; do printf '.'; done; printf ' ' diff --git a/tests/scripts/check-names.sh b/tests/scripts/check-names.sh index 5f3b5a719..7c979bcd8 100755 --- a/tests/scripts/check-names.sh +++ b/tests/scripts/check-names.sh @@ -92,7 +92,7 @@ fi diff macros identifiers | sed -n -e 's/< //p' > actual-macros for THING in actual-macros enum-consts; do - printf "Names of $THING: " + printf 'Names of %s: ' "$THING" test -r $THING BAD=$( grep -v '^MBEDTLS_[0-9A-Z_]*[0-9A-Z]$' $THING || true ) if [ "x$BAD" = "x" ]; then @@ -105,7 +105,7 @@ for THING in actual-macros enum-consts; do done for THING in identifiers; do - printf "Names of $THING: " + printf 'Names of %s: ' "$THING" test -r $THING BAD=$( grep -v '^mbedtls_[0-9a-z_]*[0-9a-z]$' $THING || true ) if [ "x$BAD" = "x" ]; then diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2cf96330b..b9652ef42 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -384,7 +384,7 @@ print_name() { fi LINE="$LINE$1" - printf "$LINE " + printf "%s " "$LINE" LEN=$(( 72 - `echo "$LINE" | wc -c` )) for i in `seq 1 $LEN`; do printf '.'; done printf ' ' @@ -662,12 +662,12 @@ run_test() { fi check_osrv_dtls - printf "# $NAME\n$SRV_CMD\n" > $SRV_OUT + printf '# %s\n%s\n' "$NAME" "$SRV_CMD" > $SRV_OUT provide_input | $SRV_CMD >> $SRV_OUT 2>&1 & SRV_PID=$! wait_server_start "$SRV_PORT" "$SRV_PID" - printf "# $NAME\n$CLI_CMD\n" > $CLI_OUT + printf '# %s\n%s\n' "$NAME" "$CLI_CMD" > $CLI_OUT eval "$CLI_CMD" >> $CLI_OUT 2>&1 & wait_client_done @@ -1877,12 +1877,12 @@ run_test "Session resume using cache, DTLS: openssl server" \ # Tests for Max Fragment Length extension if [ "$MAX_CONTENT_LEN" -lt "4096" ]; then - printf "${CONFIG_H} defines MBEDTLS_SSL_MAX_CONTENT_LEN to be less than 4096. Fragment length tests will fail.\n" + printf '%s defines MBEDTLS_SSL_MAX_CONTENT_LEN to be less than 4096. Fragment length tests will fail.\n' "${CONFIG_H}" exit 1 fi if [ $MAX_CONTENT_LEN -ne 16384 ]; then - printf "Using non-default maximum content length $MAX_CONTENT_LEN\n" + echo "Using non-default maximum content length $MAX_CONTENT_LEN" fi requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH @@ -2823,14 +2823,14 @@ MAX_IM_CA='8' MAX_IM_CA_CONFIG=$( ../scripts/config.pl get MBEDTLS_X509_MAX_INTERMEDIATE_CA) if [ -n "$MAX_IM_CA_CONFIG" ] && [ "$MAX_IM_CA_CONFIG" -ne "$MAX_IM_CA" ]; then - printf "The ${CONFIG_H} file contains a value for the configuration of\n" - printf "MBEDTLS_X509_MAX_INTERMEDIATE_CA that is different from the script’s\n" - printf "test value of ${MAX_IM_CA}. \n" - printf "\n" - printf "The tests assume this value and if it changes, the tests in this\n" - printf "script should also be adjusted.\n" - printf "\n" + cat <