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

Overflow in multiply when formatting instruction #15

Closed
5225225 opened this issue Dec 18, 2021 · 2 comments
Closed

Overflow in multiply when formatting instruction #15

5225225 opened this issue Dec 18, 2021 · 2 comments

Comments

@5225225
Copy link
Contributor

5225225 commented Dec 18, 2021

(grr, the second time i hit enter and made an issue too early, sorry if you have email notifs on!)

fn main() {
    let decoder = yaxpeax_x86::amd64::InstDecoder::default();
    let mut s = String::new();
    let i = decoder.decode_slice(&[243, 15, 30, 15]).expect("unreachable");
    i.write_to(&mut s).expect("unreachable");
}

Gives a multiply with overflow with integer overflow checks on, gives a index out of bounds panic (thankfully not unchecked indexing) in release

@5225225
Copy link
Contributor Author

5225225 commented Dec 18, 2021

The fuzzer I used to find this is just

#![no_main]
use libfuzzer_sys::fuzz_target;

fuzz_target!(|data: &[u8]| {
    let decoder = yaxpeax_x86::amd64::InstDecoder::default();
    let mut s = String::new();
    if let Ok(i) = decoder.decode_slice(data) {
        i.write_to(&mut s).expect("writing to never fail");
    }
});

@iximeow
Copy link
Owner

iximeow commented Dec 19, 2021

i see this yield a attempt to subtract with overflow, rather than multiply? the cause of this is a little deeper than just the formatting though: the nop instruction is reported as having a memory operand, and then never gets an explicit mem_size, so it keeps the default of 0. then in..

out.write_str(MEM_SIZE_STRINGS[instr.mem_size as usize - 1])

you can see why it would panic .. :D

fixed in e7dec7b.

and then i've added fuzz targets for both decoding and displaying instructions in 26e019c. i'm gonna let cargo fuzz go a while longer before cutting a new release, though, since i doubt this is the only instruction i missed when adding the explicit mem_size field.

@iximeow iximeow closed this as completed Dec 19, 2021
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

2 participants