Return None for signed values with a non-decimal timestamp - #3708
Open
eeshsaxena wants to merge 1 commit into
Open
Return None for signed values with a non-decimal timestamp#3708eeshsaxena wants to merge 1 commit into
eeshsaxena wants to merge 1 commit into
Conversation
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.
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
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.
Fixes #3701
Problem
decode_signed_valueraisesValueErrorinstead of returningNonewhen a v2 signed value has a syntactically malformed (non-decimal) timestamp field, even though the HMAC signature verified._decode_signed_value_v2does:which throws
ValueError: invalid literal for int() with base 10: b'a'.RequestHandler.get_signed_cookiedocuments that an invalid signed value returnsNone, 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 returnNoneonValueError, 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 isb"a"and asserts it decodes toNone. It raises the reportedValueErrorwithout this change and passes with it; the fullSignedValueTestsuite passes (11 tests).