Skip to content

Return None for signed values with a non-decimal timestamp - #3708

Open
eeshsaxena wants to merge 1 commit into
tornadoweb:masterfrom
eeshsaxena:fix/decode-signed-value-malformed-timestamp-3701
Open

Return None for signed values with a non-decimal timestamp#3708
eeshsaxena wants to merge 1 commit into
tornadoweb:masterfrom
eeshsaxena:fix/decode-signed-value-malformed-timestamp-3701

Conversation

@eeshsaxena

Copy link
Copy Markdown

Fixes #3701

Problem

decode_signed_value raises ValueError instead of returning None when a v2 signed value has a syntactically malformed (non-decimal) timestamp field, even though the HMAC signature verified. _decode_signed_value_v2 does:

timestamp = int(timestamp_bytes)

which throws ValueError: invalid literal for int() with base 10: b'a'.

RequestHandler.get_signed_cookie documents that an invalid signed value returns None, so a malformed cookie should be rejected, not raise into request handling.

This is not a signature-bypass: producing such an input requires the signing key. But a malformed value from a key-holding component (or during a key rotation/migration) turns into an unhandled exception rather than a clean rejection.

Fix

Wrap the int() conversion and return None on ValueError, consistent with the other rejection paths in the same function.

Test

Added SignedValueTest.test_malformed_timestamp, which builds a correctly signed v2 value whose timestamp field is b"a" and asserts it decodes to None. It raises the reported ValueError without this change and passes with it; the full SignedValueTest suite passes (11 tests).

decode_signed_value raised ValueError from int(timestamp_bytes) in
_decode_signed_value_v2 when a v2 signed value carried a syntactically
malformed (non-decimal) timestamp field, even after the HMAC signature had
verified. get_signed_cookie documents that an invalid signed value returns
None, so this path should reject rather than raise.

Wrap the int() conversion and return None on ValueError, consistent with
the surrounding checks. Add a regression test that builds a correctly
signed v2 value with a non-decimal timestamp and asserts it decodes to
None.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

decode_signed_value raises ValueError for authenticated malformed timestamps

1 participant