fix(tools/web): tolerate non-string JWT header fields in parse_token#390
Open
VoidChecksum wants to merge 2 commits into
Open
fix(tools/web): tolerate non-string JWT header fields in parse_token#390VoidChecksum wants to merge 2 commits into
VoidChecksum wants to merge 2 commits into
Conversation
Bug: JWTHeader.from_dict stores header fields verbatim from JSON, so numeric/list alg/kid/jku values cause AttributeError/TypeError in the findings block of parse_token (e.g. int.lower() crashes on alg=1). Fix: coerce alg_s/kid_s/jku_s to str before all .lower()/"in"/ .startswith() checks, preserving None-as-empty semantics for kid/jku. Regression test: test_jwt_nonstring_headers.py covers int alg, list alg, int kid, int jku, plus verifies string-path findings still fire.
Folds the unique JWT enhancement from #414 into this PR: flag ANY jku header (not only non-HTTPS — an attacker-influenced https JWKS URL is still a key-confusion vector) and add the previously-missing x5u (X.509 URL) check. Uses the string-safe coercion from this PR's non-string-header fix so non-string jku/x5u values still can't crash parse_token. Adds regression tests. Consolidates #414's jwt change; #414's http_request part is already covered by #358.
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
1 participant
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.
Defect
parse_tokeninjwt.pycalls.lower(),in, and.startswith()directly onheader.alg,header.kid, andheader.jkuafter storing them verbatim viaJWTHeader.from_dict. A JWT header is attacker/server-controlled JSON; non-string values are legal — e.g.{"alg":1},{"alg":["none"]},{"kid":1234},{"jku":999}. These all crash the tool:int.lower()→AttributeError"../" in 1234→TypeErrorint.startswith(...)→AttributeErrorThe crashes surface at two agent-facing
@toolentry points (jwt_parseintools/web/tools.pyandkg_analyze_jwtintools/research/tools.py) with no surroundingtry/except, so probing a server that emits a numericalgorkidhalts the tool entirely.Fix
Coerce
alg,kid, andjkutostrlocals (alg_s/kid_s/jku_s) before the four findings checks, treatingNoneas empty string forkid/jku. All existing string-path findings (alg=none, jku confusion, kid traversal, jku non-HTTPS) continue to fire unchanged.Regression test
New file
packages/decepticon/tests/unit/web/test_jwt_nonstring_headers.pycovers:alg=1(int) — must not raisealg=["none"](list) — must not raisekid=1234(int) — must not raisejku=999(int) — must not raisekidtraversal finding still firesjkunon-HTTPS finding still firesalg=nonefinding still firesAll 23 tests (7 new + 16 existing) pass. No interaction with in-flight PRs (PR #387 is OAuth-only and does not touch
jwt.py).