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

[GHSA-mh63-6h87-95cp] jwt-go allows excessive memory allocation during header parsing #5427

Conversation

peterbourgon
Copy link

Updates

  • CVSS v3
  • CVSS v4
  • Description
  • Severity

Comments
HTTP headers are not of arbitrary or unbounded size, they have a maximum practical length of around 10KiB. A 10KiB HTTP header consisting of nothing but . characters, when passed to strings.Split, would certainly do more work than is necessary, but would allocate at most around 200KiB, which in the context of typical JWT processing within an HTTP request is not enormous. Furthermore, those bytes would almost certainly not escape to the heap, so the impact on e.g. GC would be minimal to zero. In general, the impact of this issue to any user, even when subject to the most malicious possible input, is limited/bounded, not disproportionate to any existing load, and unlikely to have any meaningful impact on the service. I'm therefore suggesting to reduce the impact to availability from "high" to "low". But, do with this feedback what you will.

@github
Copy link
Collaborator

github commented Apr 2, 2025

Hi there @oxisto! A community member has suggested an improvement to your security advisory. If approved, this change will affect the global advisory listed at github.com/advisories. It will not affect the version listed in your project repository.

This change will be reviewed by our Security Curation Team. If you have thoughts or feedback, please share them in a comment here! If this PR has already been closed, you can start a new community contribution for this advisory

@github-actions github-actions bot changed the base branch from main to peterbourgon/advisory-improvement-5427 April 2, 2025 14:18
@shelbyc
Copy link
Contributor

shelbyc commented Apr 3, 2025

👋 Hi @peterbourgon, I can understand how @oxisto and the other maintainers of jwt-go came to the conclusion that CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H is appropriate for GHSA-mh63-6h87-95cp. Likelihood of exploitation isn't a factor in CVSS 3.1, only the severity if the vulnerability is exploited.

If @oxisto agrees and wants to change the CVSS, that's fine, but for now, CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H is an acceptable CVSS and it's OK to keep that score if that's what the maintainers prefer. 🙂

@peterbourgon
Copy link
Author

Whatever you prefer! Thanks for taking a look.

@oxisto
Copy link

oxisto commented Apr 3, 2025

I see @peterbourgon’s point. We could also go for low or medium. Unfortunately CVSS are never really a good way to describe a scenario like this.

@shelbyc
Copy link
Contributor

shelbyc commented Apr 3, 2025

@oxisto @peterbourgon If there's no proof of GHSA-mh63-6h87-95cp/CVE-2025-30204 having proof of concept code exploited in the wild, one way to get the score down would be to use CVSS 4.0. CVSS 4.0 has a metric called exploit maturity that takes into account how feasible it is to exploit a vulnerability in the wild.

Per the specification documentation, exploit maturity has three levels: unreported, for no reported exploit attempts; proof-of-concept, for public PoC code being available but no attempts to exploit reported in the wild; and attacked, for reported attempts to exploit the vulnerability in the wild.

GHSA-mh63-6h87-95cp/CVE-2025-30204 doesn't have a public PoC that I can find and I haven't seen reports of attempts to exploit it in the wild, so setting exploit maturity to unreported could get the CVSS 4.0 score down to 6.6/Medium. https://www.first.org/cvss/calculator/v4-0#CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:H/SC:N/SI:N/SA:N/E:U

@peterbourgon
Copy link
Author

And just to be clear: my feedback isn't really related to likelihood of exploitation, it's about the (worst possible) impact.

@advisory-database advisory-database bot merged commit 3c6b81f into peterbourgon/advisory-improvement-5427 Apr 10, 2025
4 checks passed
@advisory-database
Copy link
Contributor

Hi @peterbourgon! Thank you so much for contributing to the GitHub Advisory Database. This database is free, open, and accessible to all, and it's people like you who make it great. Thanks for choosing to help others. We hope you send in more contributions in the future!

@advisory-database advisory-database bot deleted the peterbourgon-GHSA-mh63-6h87-95cp branch April 10, 2025 13:02
@shelbyc
Copy link
Contributor

shelbyc commented Apr 10, 2025

@peterbourgon @oxisto My apologies for this PR suddenly getting merged. I intended to merge #5447 but accidentally caught this PR at the same time. 🤦‍♀️ I'm still OK with changing the CVSS 4.0 score in the advisory to https://www.first.org/cvss/calculator/v4-0#CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:H/SC:N/SI:N/SA:N/E:U while keeping the CVSS 3.1 score chosen by @oxisto (and with only the CVSS 3.1 score chosen by @oxisto being in the CVE record) if that's something of interest to both of you.

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