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

Limit token size to 250 KB #345

Closed
wants to merge 1 commit into from
Closed

Conversation

princekhunt
Copy link

No description provided.

@smittysmee
Copy link

Bump on this

@@ -76,6 +76,11 @@ def decrypt(jwe_str, key):
>>> jwe.decrypt(jwe_string, 'asecret128bitkey')
'Hello, World!'
"""

# limit the token size to 250 KB

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# limit the token size to 250 KB
# limit the token size to 250 KB - when this token is processed by the server, it results in significant memory allocation and processing time during decompression.

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request May 6, 2024
https://build.opensuse.org/request/show/1172135
by user dgarcia + anag+factory
- Add upstream patches:
   * CVE-2024-33663.patch, bsc#1223417, gh#mpdavis/python-jose#349
   * CVE-2024-33664.patch, bsc#1223422, gh#mpdavis/python-jose#345
   * fix-tests-ecdsa-019.patch, gh#mpdavis/python-jose#350
@alistairwatts
Copy link

Unfortunately the proposed fix just checks that the incoming uncompressed data is no more than than 250KB. I don't know what the maximum size a maliciously crafted 250KB token could expand to, but I imagine it could be significant. Some basic tests suggest that a 250KB token can expand to about 250MB.

In addition to sensibly checking the size of the compressed token, I would suggest changing the decompress function in jwe.py to use the decompress method on an instance of zlib.Decompress. The decompress method accepts a max_length which can limit the size of the decompressed data.

@smittysmee
Copy link

@princekhunt see above ☝️

@alistairwatts
Copy link

I've already opened a pull request for a more robust fix. See #352

@smittysmee
Copy link

👌

@twwildey
Copy link
Collaborator

This appears duplicative to #352 - I will close this in favor of the other PR.

@twwildey twwildey closed this May 30, 2024
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.

4 participants