hyperscale-stack / security / 38015400055

94%

master: 96%

Pull #92

github

fix(oauth2): validate PKCE verifier and challenge syntax

PKCE checked the transformation but never the RFC 7636 syntax:
pkce.Verify hashed whatever it was given, and /authorize checked the
presence and method of a code_challenge but not its shape. A client
could pair a one-character code_verifier with its S256 challenge and
have the code exchanged for tokens under every profile, and /authorize
stored challenges that no S256 transformation can produce.

The pkce package now owns the syntax. ValidVerifier encodes RFC 7636
§4.1: 43 to 128 bytes, each one unreserved, so whitespace and every
non-ASCII byte are refused. ValidChallenge encodes §4.2: a plain
challenge is a verifier, and an S256 challenge must survive a round
trip through the unpadded base64url encoding of a SHA-256 digest.
Decoding alone is not enough: encoding/base64 skips CR and LF even in
strict mode, and lax decoding ignores non-zero trailing bits. Verify
and VerifyS256 refuse a malformed verifier before the constant-time
comparison.

/authorize still checks presence first, then the method against the
profile (an absent method means plain, §4.3), and only then the
syntax, so a request breaking several rules keeps its current error; a
malformed challenge is redirected as invalid_request before the
ConsentFunc runs. /token answers invalid_grant "malformed
code_verifier" after the profile checks: §4.6 maps a failed proof to
invalid_grant, and the code, consumed before PKCE runs, stays burned.

The PKCE error descriptions no longer carry the double quote RFC 6749
§4.1.2.1 and §5.2 forbid, nor echo an unsupported method. Tests that
relied on non-compliant PKCE values now use well-formed ones, and the
security considerations and Profile docs no longer misstate which
profiles accept plain. Refs #79.

46 of 46 new or added lines in 3 files covered. (100.0%)

4675 of 4950 relevant lines covered (94.44%)

75.03 hits per line