From 15ebceca842071b0e812fb14f6d09dc99c8551e7 Mon Sep 17 00:00:00 2001 From: Dirk Date: Mon, 15 Sep 2025 15:41:43 +0200 Subject: [PATCH] Fix garbled screen when HTTP Age is not a non-negative int As suggested in https://github.com/testssl/testssl.sh/pull/2885 parsing of the server determined HTTP age var wasn't strict enough. https://www.rfc-editor.org/rfc/rfc7234#section-1.2.1 requires the variable to be a non-negative integer but testssl.sh assumed it was like that but did't check whether that really was the case. This was labled as a (potential) security problem. Potential as it didn't look exploitable after review -- the header as a whole was already sanitized. This PR fixes the typs confusion and the garbled screen by checking the variable early in run_http_header() and reset it to NaN. That will be used later in run_http_date() to raise a low severity finding. Kudos to @Tristanhx for catching this and for the suggested PR. Also, only when running in debug mode, this PR fixes that during service_detection() parts of the not-yet-sanitized header ended up on the screen. The fix just calls sanitze_http_header() for the temporary variable $TMPFILE. --- testssl.sh | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index a3e27b8..51d9d43 100755 --- a/testssl.sh +++ b/testssl.sh @@ -2503,6 +2503,8 @@ service_detection() { wait_kill $! $((HEADER_MAXSLEEP * 10)) was_killed=$? fi + # make sure that we don't have non-printable chars sneaked in -- relecant only in debug mode level 2 + sanitze_http_header $TMPFILE head $TMPFILE | grep -aq '^HTTP/' && SERVICE=HTTP [[ -z "$SERVICE" ]] && head $TMPFILE | grep -Ewaq "SMTP|ESMTP|Exim|IdeaSmtpServer|Kerio Connect|Postfix" && SERVICE=SMTP # I know some overlap here [[ -z "$SERVICE" ]] && head $TMPFILE | grep -Ewaq "POP|POP3|Gpop|OK Dovecot" && SERVICE=POP # I know some overlap here @@ -2577,7 +2579,9 @@ connectivity_problem() { fi } - +# arg1: filename (global) +# return: sanitzes arg1 +# sanitze_http_header() { # some sed implementations were sometime not fine with HTTP headers containing x0d x0a (CRLF: usual case) # Also we use tr here to remove any crtl chars which the server side offers --> possible security problem. @@ -2636,6 +2640,7 @@ run_http_header() { ((NR_HEADER_FAIL++)) fi fi + HTTP_TIME=$(awk -F': ' '/^date:/ { print $2 } /^Date:/ { print $2 }' $HEADERFILE) HTTP_AGE=$(awk -F': ' '/^[aA][gG][eE]: / { print $2 }' $HEADERFILE) if [[ ! -s $HEADERFILE ]]; then @@ -2663,6 +2668,10 @@ run_http_header() { # Populate vars for HTTP time [[ -n "$HTTP_AGE" ]] && HTTP_AGE="$(strip_lf "$HTTP_AGE")" [[ -n "$HTTP_TIME" ]] && HTTP_TIME="$(strip_lf "$HTTP_TIME")" + if [[ -n "$HTTP_AGE" ]] && [[ ! "$HTTP_AGE" =~ ^[0-9]+$ ]]; then + HTTP_AGE="NaN" + fi + debugme echo "NOW_TIME: $NOW_TIME | HTTP_AGE: $HTTP_AGE | HTTP_TIME: $HTTP_TIME" HTTP_STATUS_CODE=$(awk '/^HTTP\// { print $2 }' $HEADERFILE 2>>$ERRFILE) @@ -2793,13 +2802,20 @@ run_http_date() { outln pr_bold " HTTP Age" out " (RFC 7234) $HTTP_AGE" - fileout "HTTP_headerAge" "INFO" "$HTTP_AGE seconds" + if [[ "$HTTP_AGE" = NaN ]]; then + out ", " + # https://www.rfc-editor.org/rfc/rfc7234#section-1.2.1 + pr_svrty_low "RFC 7234, sec 1.2.1. requires numbers" + fileout "HTTP_headerAge" "LOW" "$HTTP_AGE was not a non-negative integer, see RFC 7234, sec 1.2.1." + else + fileout "HTTP_headerAge" "INFO" "$HTTP_AGE seconds" + fi fi else out "Got no HTTP time, maybe try different URL?"; fileout "$jsonID" "INFO" "Got no HTTP time, maybe try different URL?" fi - debugme tm_out ", HTTP_TIME + HTTP_AGE in epoch: $HTTP_TIME / $HTTP_AGE" + debugme tm_out ", HTTP_TIME | HTTP_AGE: $HTTP_TIME | $HTTP_AGE" outln match_ipv4_httpheader "$1" return 0