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

Buffer overflow due to integer overflow #11

Closed
caibear opened this issue Dec 18, 2023 · 6 comments
Closed

Buffer overflow due to integer overflow #11

caibear opened this issue Dec 18, 2023 · 6 comments

Comments

@caibear
Copy link

caibear commented Dec 18, 2023

[dependencies]
transpose = "0.2.2"
fn main() {
    let input = [0];
    let mut output = input;
    let width = (1 << (usize::BITS - 1)) + 1;
    let height = width;
    transpose::transpose(&input, &mut output, width, height);
}

This panics on integer overflow in debug mode but results in a segmentation fault in release mode since overflows checks are off.

Current safety checks (fail on integer overflow):

assert_eq!(input_width*input_height, input.len());
assert_eq!(input_width*input_height, output.len());

Possible new safety checks (catch integer overflow):

let area = input_width.checked_mul(input_height).expect("area overflow");
assert_eq!(area, input.len());
assert_eq!(area, output.len());

Also transpose_inplace uses similar checks but doesn't have any unsafe.

@Shnatsel
Copy link

Heads up: this issue has been included in the RustSec advisory database. It will be surfaced by tools such as cargo-audit or Dependabot from now on.

Once a fix is released to crates.io, please open a pull request to update the advisory with the patched version, or file an issue on the advisory database repository.

@torokati44
Copy link

As a result, this now soft-fails our CI over here: https://github.com/ruffle-rs/ruffle/actions/runs/7958150609/job/21722385471

Can we expect a fix for this in the near future? Would a PR with the fix suggested above help?

@ejmahler
Copy link
Owner

Thanks for reporting. The inline version doesn't use unsafe, but I replaced the raw multiplication with a checked multiplication there too.

@Shnatsel
Copy link

Thank you!

Please let me know once the fix is published to crates.io, and I'll update the security advisory with the fixed version.

@ejmahler
Copy link
Owner

https://crates.io/crates/transpose/0.2.3

Thanks to @caibear for reporting, thanks to @torokati44 and @Shnatsel for notifying me of impacts/consequences of the bug.

@Shnatsel
Copy link

Added patched version to the advisory in rustsec/advisory-db#1897. Thanks!

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

No branches or pull requests

4 participants