Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jws/jwe: split token into fixed number of parts #1308

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

drakkan
Copy link
Contributor

@drakkan drakkan commented Feb 25, 2025

this avoid to use eccessive memory when processing maliciously crafted tokens with a large number of '.' characters.

See also:

golang/go#71490
GHSA-c6gw-w398-hv78

this avoid to use eccessive memory when processing maliciously
crafted tokens with a large number of '.' characters

Signed-off-by: Nicola Murino <[email protected]>
@lestrrat
Copy link
Collaborator

@drakkan If this is done for safety, I wonder if using bytes.SplitN using n + 1 as the number of expected segments would be better?

For example

input := []byte("one.two.three")
expectedSegments := 3
split := bytes.SplitN(input, []byte{'.'}, expectedSegments+1)

if len(split) != expectedSegments {
   return ...
}

@lestrrat
Copy link
Collaborator

@drakkan Oh, I think I get it. You want to avoid creating that extra slice of bytes, which could contain potentially very large amounts of garbage. I guess in that case Count would be better.

@lestrrat lestrrat merged commit d0bb461 into lestrrat-go:develop/v3 Feb 26, 2025
15 checks passed
lestrrat pushed a commit that referenced this pull request Feb 26, 2025
this avoid to use eccessive memory when processing maliciously
crafted tokens with a large number of '.' characters

Signed-off-by: Nicola Murino <[email protected]>
lestrrat added a commit that referenced this pull request Feb 26, 2025
this avoid to use eccessive memory when processing maliciously
crafted tokens with a large number of '.' characters

Signed-off-by: Nicola Murino <[email protected]>
Co-authored-by: Nicola Murino <[email protected]>
@drakkan
Copy link
Contributor Author

drakkan commented Feb 26, 2025

@drakkan Oh, I think I get it. You want to avoid creating that extra slice of bytes, which could contain potentially very large amounts of garbage. I guess in that case Count would be better.

yes, thank you.

The Go project and also jose reported this as a security issue. I think it's a bit excessive but if possible a new version with the fix would help (I'm fine using an unreleased tag until this will happen). Thanks in advance

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.

2 participants