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

Fix a bug where xlen larger than 0x7fff was rejected #1280

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

swankjesse
Copy link
Member

We treated this short value as unsigned and it should have been treated as signed.

We treated this short value as unsigned and it should have been
treated as signed.
@swankjesse swankjesse requested a review from oldergod June 22, 2023 13:05
@swankjesse swankjesse merged commit 81bce1a into master Jun 22, 2023
12 checks passed
@swankjesse swankjesse deleted the jwilson.0622.fix_xl_xlen branch June 22, 2023 13:22
@melo0187
Copy link

Sonatype OSS Index references this PR with CVE-2023-3635.
According to NIST the CVE is awaiting analysis.

Can you please help understand whether the CVE is legitimate in that there is an exploitable attack vector?
The CVE indicates network as attack vector, but I struggle to see how an attacker could point me to the mentioned crafted GZIP archive over the network.

The dependency check we use alerts us because of our use of okio:2.8.0, which is transitively pulled in by okhttp:4.9.3 due to our dependency on spring-boot:2.7.13. Is there actually a version of okio with this fix that I could use as drop in replacement of okio:2.8.0?

@swankjesse
Copy link
Member Author

Okio 3.4.0 has the fix. An attacker would attack you by serving a malicious file and crashing your program.

@barchetta
Copy link

@swankjesse Does this bug exist in the 1.x and 2.x branches as well?

@swankjesse
Copy link
Member Author

@barchetta yes both 1.x and 2.x major versions are impacted, and upgrading to 3.4.0 will fix these.

@skjolber
Copy link

skjolber commented Aug 7, 2023

@barchetta do yo plan on backporting the fix to the 1.x and 2.x branches?

@xstefank
Copy link

I tried to backport it to 1.x - #1334.

sleberknight added a commit to kiwiproject/consul-client that referenced this pull request Aug 19, 2023
* Add okio-bom with version 3.5.0 to dependency management
* This fixes a CVE from 3.2.0 related to a type conversion error;
  see square/okio#1280
sleberknight added a commit to kiwiproject/consul-client that referenced this pull request Aug 19, 2023
* Add okio-bom with version 3.5.0 to dependency management
* This fixes a CVE from 3.2.0 related to a type conversion error;
  see square/okio#1280
@zeroflag
Copy link

Anyone can help me understand how is this CVE legit? If a malformed gzip content (where xlen is greater than 0x7fff) is handed over to okio then it'll throw an InvalidArgumentException, because xlen is interpreted as a negative number. The caller will likely catch the exception, log out and move on. The worst what can happen is an unhandled runtime exception. This is certainly a bug, but I fail to see how is this exploitable.

@TomasHofman
Copy link

I think the point is that the uncaught exception from okio could bring down the larger system which uses the okio library, thereby causing denial of service.

Obviously it depends on the larger system resiliency. It doesn't automatically mean that your project which uses affected version of okio is vulnerable - that needs to be assessed by you in the context of your project. But as they say it's better be safe than sorry...

@zeroflag
Copy link

Thanks for the response. Yes, that's my understanding too. If the caller lacks exception handling then this might kill the caller's thread, which might or might not be the main thread. But having an exception handler prevents this.

@JakeWharton
Copy link
Member

Most people don't care about understanding the impact of the bug (including the vulnerability database in which the CVE is filed), they just want the notification from automated tooling analyzing their dependency graph to stop complaining.

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.

None yet

9 participants