From 06506b371ef7bfb5aabe8fa39d50111d1f10c232 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sat, 11 Mar 2023 13:38:28 +0100 Subject: [PATCH 1/5] Make sure control chars from HTTP header don't end up in html,csv,json This addresses the bug #2330 by implementing a function which removes control characters from the file output format html,csv,json at the output. In every instance called there's a check before whether the string contains control chars, hoping it'll save a few milli seconds. A tr function is used, omitting LF. It doesn't filter the terminal output and the log file output. --- testssl.sh | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index 58a1c23..955a4fe 100755 --- a/testssl.sh +++ b/testssl.sh @@ -534,7 +534,6 @@ show_finding() { html_reserved(){ local output "$do_html" || return 0 - #sed -e 's/\&/\&/g' -e 's//\>/g' -e 's/"/\"/g' -e "s/'/\'/g" <<< "$1" output="${1//&/$'&'amp;}" output="${output///$'&'gt;}" @@ -545,8 +544,26 @@ html_reserved(){ } html_out() { + local outstr="$1" + "$do_html" || return 0 - [[ -n "$HTMLFILE" ]] && [[ ! -d "$HTMLFILE" ]] && printf -- "%b" "$1" >> "$HTMLFILE" + if [[ -n "$HTMLFILE" ]] && [[ ! -d "$HTMLFILE" ]]; then + if [[ "$outstr" =~ [[:cntrl:]] ]]; then + outstr="$(sanitize_fileout "$outstr")" + fi + printf -- "%b" "$outstr" >> "$HTMLFILE" + fi +} + +# Removes on printable chars in CSV, JSON, HTML, see #2330 +sanitize_fileout() { + tr -d '\000-\011,\013-\037' <<< "$1" +} + +# Removes on printable chars in terminal output (log files) +# We need to keep the icolor ANSI escape code, see #2330 +sanitize_termout() { + tr -d '\000-\011,\013-\032,\034-\037' <<< "$1" } # This is intentionally the same. @@ -1227,6 +1244,9 @@ fileout_json_print_parameter() { spaces=" " || \ spaces=" " if [[ -n "$value" ]] || [[ "$parameter" == finding ]]; then + if [[ "$value" =~ [[:cntrl:]] ]]; then + value="$(sanitize_fileout "$value")" + fi printf -- "%b%b%b%b" "$spaces" "\"$parameter\"" "$filler" ": \"$value\"" >> "$JSONFILE" "$not_last" && printf ",\n" >> "$JSONFILE" fi @@ -1350,12 +1370,19 @@ fileout_insert_warning() { fi } +# args: "id" "fqdn/ip" "port" "severity" "finding" "cve" "cwe" "hint" +# fileout_csv_finding() { + local finding="$5" + + if [[ "$finding" =~ [[:cntrl:]] ]]; then + finding="$(sanitize_fileout "$finding")" + fi safe_echo "\"$1\"," >> "$CSVFILE" safe_echo "\"$2\"," >> "$CSVFILE" safe_echo "\"$3\"," >> "$CSVFILE" safe_echo "\"$4\"," >> "$CSVFILE" - safe_echo "\"$5\"," >> "$CSVFILE" + safe_echo "\"$finding\"," >> "$CSVFILE" safe_echo "\"$6\"," >> "$CSVFILE" if "$GIVE_HINTS"; then safe_echo "\"$7\"," >> "$CSVFILE" From d298b41d2cdd153473c6467c9f4be4199ea9c2b5 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sat, 11 Mar 2023 14:06:47 +0100 Subject: [PATCH 2/5] add aNULL exception to codespell --- .github/workflows/codespell.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml index 1c33479..3165b94 100644 --- a/.github/workflows/codespell.yml +++ b/.github/workflows/codespell.yml @@ -13,4 +13,4 @@ jobs: - uses: codespell-project/actions-codespell@master with: skip: ca_hashes.txt,tls_data.txt,*.pem,OPENSSL-LICENSE.txt - ignore_words_list: borken,gost,ciph,ba,bloc,isnt,chello,fo,alle + ignore_words_list: borken,gost,ciph,ba,bloc,isnt,chello,fo,alle,aNULL From fab67d0cca37b7f3f6f3819e6f117d3266553f8f Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sun, 12 Mar 2023 14:00:55 +0100 Subject: [PATCH 3/5] Remove CR in server banner ... which caused a problem in t/32_isHTML_valid.t. Also the test for an empty server banner was simplified --- testssl.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index 955a4fe..8d1bd71 100755 --- a/testssl.sh +++ b/testssl.sh @@ -3129,16 +3129,18 @@ run_server_banner() { grep -ai '^Server' $HEADERFILE >$TMPFILE if [[ $? -eq 0 ]]; then serverbanner=$(sed -e 's/^Server: //' -e 's/^server: //' $TMPFILE) - if [[ "$serverbanner" == $'\n' ]] || [[ "$serverbanner" == $'\r' ]] || [[ "$serverbanner" == $'\n\r' ]] || [[ -z "$serverbanner" ]]; then + serverbanner=${serverbanner//$'\r'} + serverbanner=${serverbanner//$'\n'} + if [[ -z "$serverbanner" ]]; then outln "exists but empty string" fileout "$jsonID" "INFO" "Server banner is empty" else emphasize_stuff_in_headers "$serverbanner" fileout "$jsonID" "INFO" "$serverbanner" if [[ "$serverbanner" == *Microsoft-IIS/6.* ]] && [[ $OSSL_VER == 1.0.2* ]]; then - prln_warning " It's recommended to run another test w/ OpenSSL 1.0.1 !" + prln_warning " It's recommended to run another test w/ OpenSSL >= 1.0.1 !" # see https://github.com/PeterMosmans/openssl/issues/19#issuecomment-100897892 - fileout "${jsonID}" "WARN" "IIS6_openssl_mismatch: Recommended to rerun this test w/ OpenSSL 1.0.1. See https://github.com/PeterMosmans/openssl/issues/19#issuecomment-100897892" + fileout "${jsonID}" "WARN" "IIS6_openssl_mismatch: Recommended to rerun this test w/ OpenSSL >= 1.0.1. See https://github.com/PeterMosmans/openssl/issues/19#issuecomment-100897892" fi fi # mozilla.github.io/server-side-tls/ssl-config-generator/ From 2e33c483dd2fa1dbbe4e6dfd5ea53710b092bae7 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sun, 12 Mar 2023 14:52:11 +0100 Subject: [PATCH 4/5] remove comma in tr as it was interpreted as such --- testssl.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/testssl.sh b/testssl.sh index 8d1bd71..5d4b98c 100755 --- a/testssl.sh +++ b/testssl.sh @@ -555,15 +555,15 @@ html_out() { fi } -# Removes on printable chars in CSV, JSON, HTML, see #2330 +# Removes non-printable chars in CSV, JSON, HTML, see #2330 sanitize_fileout() { - tr -d '\000-\011,\013-\037' <<< "$1" + tr -d '\000-\011\013-\037' <<< "$1" } -# Removes on printable chars in terminal output (log files) -# We need to keep the icolor ANSI escape code, see #2330 +# Removes non-printable chars in terminal output (log files) +# We need to keep the color ANSI escape code x1b, o33, see #2330 sanitize_termout() { - tr -d '\000-\011,\013-\032,\034-\037' <<< "$1" + tr -d '\000-\011\013-\032\034-\037' <<< "$1" } # This is intentionally the same. From cacd8c57b10c455c76960692e98b293c037f90af Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sun, 12 Mar 2023 15:09:24 +0100 Subject: [PATCH 5/5] Add variable htmlfile + filter GOST message ... which is needed for newer LibreSSL/OpenSSL versions --- t/32_isHTML_valid.t | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/t/32_isHTML_valid.t b/t/32_isHTML_valid.t index becef33..0524372 100755 --- a/t/32_isHTML_valid.t +++ b/t/32_isHTML_valid.t @@ -15,22 +15,23 @@ my $out=""; my $html=""; my $debughtml=""; my $edited_html=""; -my $check2run="--ip=one --ids-friendly --color 0 --htmlfile tmp.html"; +my $htmlfile="tmp.html"; +my $check2run="--ip=one --ids-friendly --color 0 --htmlfile $htmlfile"; my $diff=""; die "Unable to open $prg" unless -f $prg; printf "\n%s\n", "Doing HTML output checks"; -unlink 'tmp.html'; +unlink $htmlfile; #1 printf "%s\n", " .. running $prg against \"$uri\" to create HTML and terminal outputs (may take ~2 minutes)"; # specify a TERM_WIDTH so that the two calls to testssl.sh don't create HTML files with different values of TERM_WIDTH $out = `TERM_WIDTH=120 $prg $check2run $uri`; -$html = `cat tmp.html`; +$html = `cat $htmlfile`; # $edited_html will contain the HTML with formatting information removed in order to compare against terminal output # Start by removing the HTML header. -$edited_html = `tail -n +11 tmp.html`; -unlink 'tmp.html'; +$edited_html = `tail -n +11 $htmlfile`; +unlink $htmlfile; # Remove the HTML footer $edited_html =~ s/\n\<\/pre\>\n\<\/body\>\n\<\/html\>//; @@ -51,12 +52,13 @@ $tests++; $diff = diff \$edited_html, \$out; printf "\n%s\n", "$diff"; + #2 printf "\n%s\n", " .. running again $prg against \"$uri\", now with --debug 4 to create HTML output (may take another ~2 minutes)"; # Redirect stderr to /dev/null in order to avoid some unexplained "date: invalid date" error messages $out = `TERM_WIDTH=120 $prg $check2run --debug 4 $uri 2> /dev/null`; -$debughtml = `cat tmp.html`; -unlink 'tmp.html'; +$debughtml = `cat $htmlfile`; +unlink $htmlfile; # Remove date information from the Start and Done banners in the two HTML files, since they were created at different times $html =~ s/Start 2[0-9][0-9][0-9]-[0-3][0-9]-[0-3][0-9] [0-2][0-9]:[0-5][0-9]:[0-5][0-9]/Start XXXX-XX-XX XX:XX:XX/; @@ -72,6 +74,7 @@ $debughtml =~ s/HTTP clock skew \+?-?[0-9]* /HTTP clock skew $debughtml =~ s/ Pre-test: .*\n//g; $debughtml =~ s/.*OK: below 825 days.*\n//g; $debughtml =~ s/.*DEBUG:.*\n//g; +$debughtml =~ s/No engine or GOST support via engine with your.*\n//g; cmp_ok($debughtml, "eq", $html, "HTML file created with --debug 4 matches HTML file created without --debug"); $tests++;