Fixes an integer overflow in a helper class

[kaczmarczyck]

This was not exploitable as a vulnerability as far as I know. All inputs to the function are coming from configs, and are never user inputs.

The new unit test would have panicked without the fix.

[gemini-code-assist]

Summary of Changes

Hello @kaczmarczyck, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a potential integer overflow within the find_address helper method of the Partition struct. While not identified as an exploitable vulnerability, the change enhances the robustness of memory address calculations by utilizing Rust's checked_add function. This ensures that addition operations near the maximum value of usize gracefully return None instead of panicking or producing incorrect results, as validated by a newly introduced unit test.

Highlights

• Integer Overflow Fix: The find_address method in Partition now uses checked_add to safely calculate memory addresses, preventing potential integer overflows that could lead to panics or incorrect behavior.
• New Unit Test: A new unit test, partition_find_address_overflow, has been added to specifically test and confirm the correct handling of potential integer overflows in address calculations, ensuring None is returned as expected.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly fixes a potential integer overflow in find_address by using checked_add. The accompanying unit test effectively demonstrates the issue and verifies the fix. My review includes a minor suggestion to improve the readability of the new test case by adding a comment that explains why the chosen values trigger the overflow.

[coveralls]

coverage: 97.189% (+0.001%) from 97.188%
when pulling 4dccf83 on kaczmarczyck:integer-overflow
into 016e81c on google:develop.

[ia0]

I don't think this test is useful. We should instead fix ModRange::new() to panic if start + length overflows (or return an error but that would mean more changes).

[kaczmarczyck]

Some unit tests for ModRange check slices that overflow. I'd have to check if we rely on this behavior somewhere, or it makes something more robust. But the panic in ModRange would be a change in working existing behavior.

[ia0]

This PR is also changing the existing behavior in almost exactly the same way. If start + length overflow for a range in find_address then it will overflow the same way in ModRange::new(). Also the documentation of ModRange is clear that it represents [start, start + length) which is nonsensical in case of overflow.

[kaczmarczyck]

ModRange is used more widely than Partition. So we can fix partition and make it robust, without touching ModRange.

ModRange::new does not overflow, so I think you mean it should have an end that wouldn't overflow?

    pub fn new(start: usize, length: usize) -> ModRange {
        ModRange { start, length }
    }

I'm not sure why a ModRange that doesn't have an end within usize is not acceptable as is. Its public functions seem to not contradict this, e.g. intersects_range? Can't it represent [start, start + length) as the interval you know from Maths?

[kaczmarczyck]

Discussed offline, not exploitable so not worth the change.