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: Add protection against a possible null dereference issue #1258

Conversation

munahaf
Copy link
Contributor

@munahaf munahaf commented Sep 23, 2023

In file: SeekableByteChannelPrefetcher.java, class: SeekableByteChannelPrefetcher, there is a method fetch in which there is a potential Null pointer dereference. This may throw an unexpected null pointer exception which, if unhandled, may crash the program.

In line 345 of SeekableByteChannelPrefetcher.java, we have

    WorkUnit candidate=fetching;

Here fetching may be null. There is already hint at that in the null check in line 342. If fetching is null, we would have gone into the call to ensureFetching. Now this part is a grey area. The method is suggesting that it will ensure that fetching will have a value, but apparently, there may be ways to return from the function without updating the value of fetching. An example is line 309.

Then again, In line 359, the variable candidate gets assigned.

    candidate=fetching;

At that point the fetching may be null (from line 349). So, in the next line when we have:

    buf=candidate.getBuf();

we may encounter a null pointer dereference.

I introduced a null check in the appropriate path. Please review.

Again the grey area is whether ensureFetching is always guaranteed to return a non-null value for fetching.

Sponsorship and Support:

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.

@munahaf munahaf requested a review from a team as a code owner September 23, 2023 07:04
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: storage Issues related to the googleapis/java-storage-nio API. labels Sep 23, 2023
Copy link
Contributor

@sydney-munro sydney-munro left a comment

Choose a reason for hiding this comment

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

good catch, thanks!

@sydney-munro sydney-munro changed the title Add protection against a possible null dereference issue fix: Add protection against a possible null dereference issue Oct 9, 2023
@sydney-munro sydney-munro added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2023
@sydney-munro sydney-munro added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 9, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 9, 2023
@sydney-munro sydney-munro merged commit ade173b into googleapis:main Oct 9, 2023
18 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage-nio API. size: xs Pull request size is extra small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants