Skip to content

Commit 828af39

Browse files
committed
Backporting Make sure control chars from HTTP header don't end up in html,csv,json
This is for 3.0. For 3.1dev, see #2332 . This PR addresses the bug #2330 by implementing a function which removes control characters from the file output format html,csv,json in 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, yet. It provides a function though which is not being called.
1 parent 3445366 commit 828af39

3 files changed

Lines changed: 46 additions & 14 deletions

File tree

.github/workflows/codespell.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@ jobs:
1313
- uses: codespell-project/actions-codespell@master
1414
with:
1515
skip: ca_hashes.txt,tls_data.txt,*.pem,OPENSSL-LICENSE.txt,.git
16-
ignore_words_list: borken,gost,ciph,ba,bloc,isnt,chello,fo,alle,nmake,aNULL
16+
ignore_words_list: borken,gost,ciph,ba,bloc,isnt,chello,fo,alle,nmake,anull

t/08_isHTML_valid.t

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,26 @@ my $prg="./testssl.sh";
1313
my $uri="heise.de";
1414
my $out="";
1515
my $html="";
16+
my $htmlfile="tmp.html";
1617
my $debughtml="";
1718
my $edited_html="";
18-
my $check2run="--ip=one --color 0 --htmlfile tmp.html";
19+
my $check2run="--ip=one --color 0 --htmlfile $htmlfile";
1920
my $diff="";
2021

2122
die "Unable to open $prg" unless -f $prg;
2223

2324
printf "\n%s\n", "Doing HTML output checks";
24-
unlink 'tmp.html';
25+
unlink $htmlfile;
2526

2627
#1
2728
printf "%s\n", " .. running $prg against \"$uri\" to create HTML and terminal outputs (may take ~2 minutes)";
2829
# specify a TERM_WIDTH so that the two calls to testssl.sh don't create HTML files with different values of TERM_WIDTH
2930
$out = `TERM_WIDTH=120 $prg $check2run $uri`;
30-
$html = `cat tmp.html`;
31+
$html = `cat $htmlfile`;
3132
# $edited_html will contain the HTML with formatting information removed in order to compare against terminal output
3233
# Start by removing the HTML header.
33-
$edited_html = `tail -n +11 tmp.html`;
34-
unlink 'tmp.html';
34+
$edited_html = `tail -n +11 $htmlfile`;
35+
unlink $htmlfile;
3536

3637
# Remove the HTML footer
3738
$edited_html =~ s/\n\<\/pre\>\n\<\/body\>\n\<\/html\>//;
@@ -49,12 +50,13 @@ $edited_html =~ s/&apos;/'/g;
4950
cmp_ok($edited_html, "eq", $out, "HTML file matches terminal output");
5051
$tests++;
5152

53+
5254
#2
5355
printf "\n%s\n", " .. running again $prg against \"$uri\", now with --debug 4 to create HTML output (may take another ~2 minutes)";
5456
# Redirect stderr to /dev/null in order to avoid some unexplained "date: invalid date" error messages
5557
$out = `TERM_WIDTH=120 $prg $check2run --debug 4 $uri 2> /dev/null`;
56-
$debughtml = `cat tmp.html`;
57-
unlink 'tmp.html';
58+
$debughtml = `cat $htmlfile`;
59+
unlink $htmlfile;
5860

5961
# Remove date information from the Start and Done banners in the two HTML files, since they were created at different times
6062
$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/;
@@ -70,6 +72,7 @@ $debughtml =~ s/HTTP clock skew \+?-?[0-9]* /HTTP clock skew
7072
$debughtml =~ s/ Pre-test: .*\n//g;
7173
$debughtml =~ s/.*OK: below 825 days.*\n//g;
7274
$debughtml =~ s/.*DEBUG:.*\n//g;
75+
$debughtml =~ s/No engine or GOST support via engine with your.*\n//g;
7376

7477
cmp_ok($debughtml, "eq", $html, "HTML file created with --debug 4 matches HTML file created without --debug");
7578
$tests++;

testssl.sh

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,6 @@ show_finding() {
490490
html_reserved(){
491491
local output
492492
"$do_html" || return 0
493-
#sed -e 's/\&/\&amp;/g' -e 's/</\&lt;/g' -e 's/>/\&gt;/g' -e 's/"/\&quot;/g' -e "s/'/\&apos;/g" <<< "$1"
494493
output="${1//&/$'&'amp;}"
495494
output="${output//</$'&'lt;}"
496495
output="${output//>/$'&'gt;}"
@@ -501,8 +500,26 @@ html_reserved(){
501500
}
502501

503502
html_out() {
503+
local outstr="$1"
504+
504505
"$do_html" || return 0
505-
[[ -n "$HTMLFILE" ]] && [[ ! -d "$HTMLFILE" ]] && printf -- "%b" "$1" >> "$HTMLFILE"
506+
if [[ -n "$HTMLFILE" ]] && [[ ! -d "$HTMLFILE" ]]; then
507+
if [[ "$outstr" =~ [[:cntrl:]] ]]; then
508+
outstr="$(sanitize_fileout "$outstr")"
509+
fi
510+
printf -- "%b" "$outstr" >> "$HTMLFILE"
511+
fi
512+
}
513+
514+
# Removes non-printable chars in CSV, JSON, HTML, see #2330
515+
sanitize_fileout() {
516+
tr -d '\000-\011\013-\037' <<< "$1"
517+
}
518+
519+
# Removes non-printable chars in terminal output (log files)
520+
# We need to keep the color ANSI escape code x1b, o33, see #2330
521+
sanitize_termout() {
522+
tr -d '\000-\011\013-\032\034-\037' <<< "$1"
506523
}
507524

508525
# This is intentionally the same.
@@ -806,6 +823,9 @@ fileout_json_print_parameter() {
806823
spaces=" " || \
807824
spaces=" "
808825
if [[ -n "$value" ]] || [[ "$parameter" == finding ]]; then
826+
if [[ "$value" =~ [[:cntrl:]] ]]; then
827+
value="$(sanitize_fileout "$value")"
828+
fi
809829
printf -- "%b%b%b%b" "$spaces" "\"$parameter\"" "$filler" ": \"$value\"" >> "$JSONFILE"
810830
"$not_last" && printf ",\n" >> "$JSONFILE"
811831
fi
@@ -931,12 +951,19 @@ fileout_insert_warning() {
931951
fi
932952
}
933953

954+
# args: "id" "fqdn/ip" "port" "severity" "finding" "cve" "cwe" "hint"
955+
#
934956
fileout_csv_finding() {
957+
local finding="$5"
958+
959+
if [[ "$finding" =~ [[:cntrl:]] ]]; then
960+
finding="$(sanitize_fileout "$finding")"
961+
fi
935962
safe_echo "\"$1\"," >> "$CSVFILE"
936963
safe_echo "\"$2\"," >> "$CSVFILE"
937964
safe_echo "\"$3\"," >> "$CSVFILE"
938965
safe_echo "\"$4\"," >> "$CSVFILE"
939-
safe_echo "\"$5\"," >> "$CSVFILE"
966+
safe_echo "\"$finding\"," >> "$CSVFILE"
940967
safe_echo "\"$6\"," >> "$CSVFILE"
941968
if "$GIVE_HINTS"; then
942969
safe_echo "\"$7\"," >> "$CSVFILE"
@@ -2855,16 +2882,18 @@ run_server_banner() {
28552882
grep -ai '^Server' $HEADERFILE >$TMPFILE
28562883
if [[ $? -eq 0 ]]; then
28572884
serverbanner=$(sed -e 's/^Server: //' -e 's/^server: //' $TMPFILE)
2858-
if [[ "$serverbanner" == $'\n' ]] || [[ "$serverbanner" == $'\r' ]] || [[ "$serverbanner" == $'\n\r' ]] || [[ -z "$serverbanner" ]]; then
2885+
serverbanner=${serverbanner//$'\r'}
2886+
serverbanner=${serverbanner//$'\n'}
2887+
if [[ -z "$serverbanner" ]]; then
28592888
outln "exists but empty string"
28602889
fileout "$jsonID" "INFO" "Server banner is empty"
28612890
else
28622891
emphasize_stuff_in_headers "$serverbanner"
28632892
fileout "$jsonID" "INFO" "$serverbanner"
28642893
if [[ "$serverbanner" == *Microsoft-IIS/6.* ]] && [[ $OSSL_VER == 1.0.2* ]]; then
2865-
prln_warning " It's recommended to run another test w/ OpenSSL 1.0.1 !"
2894+
prln_warning " It's recommended to run another test w/ OpenSSL >= 1.0.1 !"
28662895
# see https://github.com/PeterMosmans/openssl/issues/19#issuecomment-100897892
2867-
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"
2896+
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"
28682897
fi
28692898
fi
28702899
# mozilla.github.io/server-side-tls/ssl-config-generator/

0 commit comments

Comments
 (0)