HTTP Clock Skew Invalid Age header crash fix#2885
Closed
Tristanhx wants to merge 1 commit intotestssl:3.3devfrom
Closed
HTTP Clock Skew Invalid Age header crash fix#2885Tristanhx wants to merge 1 commit intotestssl:3.3devfrom
Tristanhx wants to merge 1 commit intotestssl:3.3devfrom
Conversation
…ains non-numerical values. This commit adds a check via regex for this value and removes the variable if it is invalid as if it was not given.
Collaborator
|
Good catch |
drwetter
reviewed
Sep 15, 2025
| if [[ -n "$HTTP_AGE" && ! "$HTTP_AGE" =~ ^[0-9]+$ ]]; then | ||
| pr_bold " HTTP Age Header " | ||
| out "Age header has invalid value: $HTTP_AGE (treated as 0)"; | ||
| unset HTTP_AGE |
Collaborator
There was a problem hiding this comment.
unsetting HTTP_AGE is not equaly setting it to zero.
Collaborator
|
Not sure whether I like this fix. Probably it's better to set it to NaN (here or in You rather want me to fix that? |
drwetter
added a commit
that referenced
this pull request
Sep 15, 2025
As suggested in #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.
13 tasks
Collaborator
|
Thanks @Tristanhx , for the finding and the PR is much appreciated! 👍 |
drwetter
added a commit
that referenced
this pull request
Sep 15, 2025
As suggested in #2885 parsing of the server determined HTTP age var wasn't strict enough, this is a backport for 3.2. 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. For 3.2 sanitze_http_header() had to be modified to accept an argument and the callers needed to be changed.
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes
HTTP_clock_skew breaks execution of rest of script if Age header contains non-numerical values. This commit adds a check via regex for this value and removes the variable if it is invalid as if it was not given.
The run_http_date() function tries to run
difftime=$((HTTP_TIME + HTTP_AGE - NOW_TIME))When the Age header is missing bash will automatically substitute with 0, but we found it sometimes occurs that the age header contains an entire date like the Date header. This will lead to an invalid expression and the script will halt.
The fix is checking if the Age header (if it exists) is purely numerical. If it is not a new heading and message is added to the output (but not the file output) and the HTTP_AGE variable is unset. This prevents the error that halts the script and allows the script to continue as if the Age header was not given.
What is your pull request about?
If it's a code change please check the boxes which are applicable
help()