add protection to count token segments ahead of parsing#2233
Conversation
Fixes: GHSA-f83f-xpx7-ffpw Signed-off-by: Bob Callaway <bcallaway@google.com>
| return "", fmt.Errorf("oidc: malformed jwt, token must have 3 parts") | ||
| } | ||
|
|
||
| parts := strings.SplitN(token, ".", 3) |
There was a problem hiding this comment.
No need to return a slice of strings when you need to work with the header of a JWT in this function.
header := token[:strings.Index(token, ".")], or say strings.Cut(token, ".") will do their job.
There was a problem hiding this comment.
I also suggest using https://github.com/go-jose/go-jose/tree/main/json to parse the header, package github.com/coreos/go-oidc/v3 uses it through the go-jose package already in this project.
https://github.com/go-jose/go-jose/tree/main/json handles duplicate keys in the right way and doesn't do case insensitive matching (before you migrate to https://pkg.go.dev/encoding/json/v2).
| parts := strings.Split(token, ".") | ||
| if len(parts) != 3 { | ||
| return "", fmt.Errorf("oidc: malformed jwt, expected 3 parts got %d", len(parts)) | ||
| if strings.Count(token, ".") != 2 { |
There was a problem hiding this comment.
You might want to be a bit more robust and handle not only regular JWTs but also SD JWTs, which have recently been graduated to a proposed standard (RFC 9901).
An SD JWT has two "." or four "." in those cases when it includes a SD-JWT+KB proof (a cryptographic key binding proof).
|
Any plans to back-port this fix to 1.7 and 1.6? Some projects could benefit from it, like kyverno |
|
There are no breaking changes between 1.6 and 1.8 that would prohibit Kyverno from updating its indirect dependency, and I see that Kyverno is already using Go 1.25 so that shouldn't be a blocker either. Note that this impacts deployments only, not client code. |
|
You are right! Thanks for the feedback. |
Count number of separators before calling Split to minimize memory allocation risk
Fixes: GHSA-f83f-xpx7-ffpw