Offset checking

[pinkforest]

Hi @vincenthouyi Would you mind commenting on the offset checking

rustsec/advisory-db#1450

There was some fuzzing done and has found potential issues

I raised originally in the wrong repo

cole14/rust-elf#23

Thanks!

[vincenthouyi]

Hi @pinkforest, what exactly should I do in the thread?
I don't have too much time to update the repo recently. If you are in hurry, you can raise PR and I would review and merge it as soon as possible.

[atulkharerivos]

@pinkforest @vincenthouyi I can certainly try and raise a PR. Note that I will be traveling internationally for the next few days, so it might be a little delayed.

[pinkforest]

@atulkharerivos any idea of the upcoming fix if you would have a moment to work on this ? thanks

[atulkharerivos]

Thanks for jogging my memory. Sorry, I was on vacation and completely forgot about it after that...will take a look again.

…

On Sun, Jan 15, 2023 at 12:18 AM pinkforest ***@***.***> wrote: @atulkharerivos <https://github.com/atulkharerivos> any idea of the upcoming fix if you would have a moment to work on this ? thanks — Reply to this email directly, view it on GitHub <#11 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ2XZGCSXHECU6UM3O266MTWSOXDPANCNFSM6AAAAAARTTFFAU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[vincenthouyi]

Hi @pinkforest @atulkharerivos , I just pushed a new version in 3ec9935 to solve crashes I found in fuzz test. Please take a look and tell me if any further crashes you find.

[atulkharerivos]

Vincent, @vincenthouyi/elf_rs ***@***.***> Thanks a lot for looking into the issue. With version 0.3, we see a ~70% reduction in the total number of crashes, and the remaining 30% appear to be panics that resulted from invalid ELF headers. As I see it, it's perfectly OK to panic when processing invalid input rather than risk continuing program execution. Some implementations may choose to return a Result<> to the caller, but it's an API choice made by the designer of the library. @pinkforest, Please feel free to mark this issue as resolved. Thanks!!!

…

On Sun, Jan 15, 2023 at 7:23 PM Vincent Hou ***@***.***> wrote: Hi @pinkforest <https://github.com/pinkforest> @atulkharerivos <https://github.com/atulkharerivos> , I just pushed a new version in 3ec9935 <3ec9935> to solve crashes I found in fuzz test. Please take a look and tell me if any further crashes you find. — Reply to this email directly, view it on GitHub <#11 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ2XZGACL2466ZFA6RVPBTTWSS5LRANCNFSM6AAAAAARTTFFAU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[vincenthouyi]

Hi @atulkharerivos, Can you give me your sample or command to reproduce the crashes on invalid ELF headers? I can try to fix them in the future releases as well. Thanks!

[atulkharerivos]

Sure, here are a few sample files that were generated by AFL [2]. To test, you can use the following program with your crate (replace the line in *bold* with the actual path to the downloaded file in question) [1]. If you want to generate your own test input, use the instructions in [3], and comment out the code in *italics* and *remove* the code in *bold*. Do ping me if you have questions or run into issues. [1]: use std::io::Read; use std::fs::File; use std::env; */** *#[macro_use]extern crate afl;* **/* use elf_rs::*; fn main() { * /*fuzz!(|data : &[u8]| { let _ = from_elf_buffer(data); });*/* read_elf(&" *id:000002,sig:06,src:000072,time:1510,execs:43776,op:havoc,rep:16* ".to_string()); } fn read_elf(filename: &String) { let mut elf_file = File::open(filename).expect("open file failed"); let mut elf_buf = Vec::<u8>::new(); elf_file.read_to_end(&mut elf_buf).expect("read file failed"); from_elf_buffer(&elf_buf); } fn from_elf_buffer(elf_buf: &[u8]) { let elf = Elf::from_bytes(elf_buf).expect("load elf file failed"); println!("{:?} header: {:?}", elf, elf.elf_header()); for p in elf.program_header_iter() { println!("{:x?}", p); } for s in elf.section_header_iter() { println!("{:x?}", s); } let s = elf.lookup_section(b".text"); println!("s {:?}", s); } [2]: https://www.dropbox.com/s/i3qz7j26aizv7cc/id%3A000000%2Csig%3A06%2Csrc%3A000000%2Ctime%3A120%2Cexecs%3A3859%2Cop%3Ahavoc%2Crep%3A4?dl=0 https://www.dropbox.com/s/c8pcillxgweey4a/id%3A000001%2Csig%3A06%2Csrc%3A000000%2Ctime%3A340%2Cexecs%3A10222%2Cop%3Ahavoc%2Crep%3A8?dl=0 https://www.dropbox.com/s/j2kcrof3qaunn38/id%3A000002%2Csig%3A06%2Csrc%3A000072%2Ctime%3A1510%2Cexecs%3A43776%2Cop%3Ahavoc%2Crep%3A16?dl=0 https://www.dropbox.com/s/fjkzyjfny6047gv/id%3A000003%2Csig%3A06%2Csrc%3A000072%2Ctime%3A1519%2Cexecs%3A44152%2Cop%3Ahavoc%2Crep%3A16?dl=0 https://www.dropbox.com/s/3gosrlr6tun2vh1/id%3A000004%2Csig%3A06%2Csrc%3A000072%2B000135%2Ctime%3A2142%2Cexecs%3A68881%2Cop%3Asplice%2Crep%3A16?dl=0 https://www.dropbox.com/s/pm2ewea8n3hmaq2/id%3A000005%2Csig%3A06%2Csrc%3A000161%2Ctime%3A2154%2Cexecs%3A69192%2Cop%3Ahavoc%2Crep%3A4?dl=0 [3]: The setup is trivial and requires a single command: cargo install afl To begin fuzz testing, create a new cargo project that references the crates to be tested, and add the following to the Cargo.toml dependencies: [dependencies] afl = "*" Add the following to main.rs: #[macro_use] extern crate afl; The fuzzing mechanism uses a closure with the fuzz! macro, and the input is a closure that accepts an &[u8] slice, e.g.: fn main() { fuzz!(|data : &[u8]| { let _ = elf_reader(data); }); } In the above, elf_reader() is a function that exercises functionality for the crates under test using the slice as the input. The idea is that the initial input for the slice uses valid data, which is then progressively mutated to expose issues. The test target can be built using the following command: cargo afl build Ater building, create the “in” and “out” directories (these are just arbitrary names, and can be changed to anything else). The “in” directory must contain at least one file with valid input for the closure in fuzz!, and the size must be 1MB or less. An illustration of a test run appears below: cargo afl build mkdir in mkdir out # Copy a valid ELF file into the in folder cp <blah> in/. # Start fuzzing cargo afl fuzz -i in -o out target/debug/<binary> Assuming everything is fine, the fuzzing will run indefinitely, and will generate output that looks like the following. Note that any text in orange indicates that there’s some issue with the setup:

…

On Mon, Jan 16, 2023 at 5:02 PM Vincent Hou ***@***.***> wrote: Hi @atulkharerivos <https://github.com/atulkharerivos>, Can you give me your sample or command to reproduce the crashes on invalid ELF headers? I can try to fix them in the future releases as well. Thanks! — Reply to this email directly, view it on GitHub <#11 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ2XZGBV3NLNFV2UBGD5TIDWSXVR3ANCNFSM6AAAAAARTTFFAU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[vincenthouyi]

Hi @atulkharerivos, your tests panics because the program asserts reading ELF file always successes as in this line:
let elf = Elf::from_bytes(elf_buf).expect("load elf file failed");
where Elf::from_bytes returns an Err due to reading an invalid ELF magic, ELF class etc. It's the caller's responsibility to handle these errors. A panic is expected when you call expect.
This is the result I run the example readelf program with your inputs:

➜  elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000000,sig\ 06,src\ 000000,time\ 120,execs\ 3859,op\ havoc,rep\ 4 
   Compiling elf_rs v0.3.0 (/Users/vincenthou/git_repo/elf_rs)
    Finished dev [unoptimized + debuginfo] target(s) in 1.04s
     Running `target/debug/examples/readelf './tests/data/id 000000,sig 06,src 000000,time 120,execs 3859,op havoc,rep 4'`
failed to extract elf file ./tests/data/id 000000,sig 06,src 000000,time 120,execs 3859,op havoc,rep 4: InvalidMagic
Error: ()
➜  elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000001,sig\ 06,src\ 000000,time\ 340,execs\ 10222,op\ havoc,rep\ 8 
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/examples/readelf './tests/data/id 000001,sig 06,src 000000,time 340,execs 10222,op havoc,rep 8'`
failed to extract elf file ./tests/data/id 000001,sig 06,src 000000,time 340,execs 10222,op havoc,rep 8: InvalidClass
Error: ()
➜  elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000002,sig\ 06,src\ 000072,time\ 1510,execs\ 43776,op\ havoc,rep\ 16 
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `target/debug/examples/readelf './tests/data/id 000002,sig 06,src 000072,time 1510,execs 43776,op havoc,rep 16'`
failed to extract elf file ./tests/data/id 000002,sig 06,src 000072,time 1510,execs 43776,op havoc,rep 16: BufferTooShort
Error: ()
➜  elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000003,sig\ 06,src\ 000072,time\ 1519,execs\ 44152,op\ havoc,rep\ 16 
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/examples/readelf './tests/data/id 000003,sig 06,src 000072,time 1519,execs 44152,op havoc,rep 16'`
failed to extract elf file ./tests/data/id 000003,sig 06,src 000072,time 1519,execs 44152,op havoc,rep 16: BufferTooShort
Error: ()
➜  elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000004,sig\ 06,src\ 000072+000135,time\ 2142,execs\ 68881,op\ splice,rep\ 16 
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/examples/readelf './tests/data/id 000004,sig 06,src 000072+000135,time 2142,execs 68881,op splice,rep 16'`
failed to extract elf file ./tests/data/id 000004,sig 06,src 000072+000135,time 2142,execs 68881,op splice,rep 16: BufferTooShort
Error: ()
➜  elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000005,sig\ 06,src\ 000161,time\ 2154,execs\ 69192,op\ havoc,rep\ 4         
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/examples/readelf './tests/data/id 000005,sig 06,src 000161,time 2154,execs 69192,op havoc,rep 4'`
failed to extract elf file ./tests/data/id 000005,sig 06,src 000161,time 2154,execs 69192,op havoc,rep 4: BufferTooShort
Error: ()

[atulkharerivos]

My apologies...I wasn't paying attention to where the panic was coming from, and had assumed that there were internal asserts to panic in case of invalid input. I changed *let elf = Elf::from_bytes(elf_buf).expect("load elf file failed");* to* if let OK(elf) = ... *to guard against it, and the fuzzing is working with zero crashes. I will reply to the original message noting my incorrect testing....thanks!!!

…

On Tue, Jan 17, 2023 at 4:14 AM Vincent Hou ***@***.***> wrote: Hi @atulkharerivos <https://github.com/atulkharerivos>, your tests panics because the program asserts reading ELF file always successes as in this line: let elf = Elf::from_bytes(elf_buf).expect("load elf file failed"); where the panic is expected because you can't read an ELF file without a valid ELF magic, a ELF class, etc. This is the result I run the example readelf program with your inputs: ➜ elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000000,sig\ 06,src\ 000000,time\ 120,execs\ 3859,op\ havoc,rep\ 4 Compiling elf_rs v0.3.0 (/Users/vincenthou/git_repo/elf_rs) Finished dev [unoptimized + debuginfo] target(s) in 1.04s Running `target/debug/examples/readelf './tests/data/id 000000,sig 06,src 000000,time 120,execs 3859,op havoc,rep 4'` failed to extract elf file ./tests/data/id 000000,sig 06,src 000000,time 120,execs 3859,op havoc,rep 4: InvalidMagic Error: () ➜ elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000001,sig\ 06,src\ 000000,time\ 340,execs\ 10222,op\ havoc,rep\ 8 Finished dev [unoptimized + debuginfo] target(s) in 0.04s Running `target/debug/examples/readelf './tests/data/id 000001,sig 06,src 000000,time 340,execs 10222,op havoc,rep 8'` failed to extract elf file ./tests/data/id 000001,sig 06,src 000000,time 340,execs 10222,op havoc,rep 8: InvalidClass Error: () ➜ elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000002,sig\ 06,src\ 000072,time\ 1510,execs\ 43776,op\ havoc,rep\ 16 Finished dev [unoptimized + debuginfo] target(s) in 0.06s Running `target/debug/examples/readelf './tests/data/id 000002,sig 06,src 000072,time 1510,execs 43776,op havoc,rep 16'` failed to extract elf file ./tests/data/id 000002,sig 06,src 000072,time 1510,execs 43776,op havoc,rep 16: BufferTooShort Error: () ➜ elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000003,sig\ 06,src\ 000072,time\ 1519,execs\ 44152,op\ havoc,rep\ 16 Finished dev [unoptimized + debuginfo] target(s) in 0.03s Running `target/debug/examples/readelf './tests/data/id 000003,sig 06,src 000072,time 1519,execs 44152,op havoc,rep 16'` failed to extract elf file ./tests/data/id 000003,sig 06,src 000072,time 1519,execs 44152,op havoc,rep 16: BufferTooShort Error: () ➜ elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000004,sig\ 06,src\ 000072+000135,time\ 2142,execs\ 68881,op\ splice,rep\ 16 Finished dev [unoptimized + debuginfo] target(s) in 0.03s Running `target/debug/examples/readelf './tests/data/id 000004,sig 06,src 000072+000135,time 2142,execs 68881,op splice,rep 16'` failed to extract elf file ./tests/data/id 000004,sig 06,src 000072+000135,time 2142,execs 68881,op splice,rep 16: BufferTooShort Error: () ➜ elf_rs git:(master) ✗ cargo run --example readelf ./tests/data/id\ 000005,sig\ 06,src\ 000161,time\ 2154,execs\ 69192,op\ havoc,rep\ 4 Finished dev [unoptimized + debuginfo] target(s) in 0.03s Running `target/debug/examples/readelf './tests/data/id 000005,sig 06,src 000161,time 2154,execs 69192,op havoc,rep 4'` failed to extract elf file ./tests/data/id 000005,sig 06,src 000161,time 2154,execs 69192,op havoc,rep 4: BufferTooShort Error: () — Reply to this email directly, view it on GitHub <#11 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ2XZGC7PG6WQBEHW5Y36N3WS2EKPANCNFSM6AAAAAARTTFFAU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[atulkharerivos]

@pinkforest, @vincenthouyi/elf_rs ***@***.***> pointed out that my test was flawed, and that the panics were coming from an .expect() based on the ELF parsing. After changing this to *if let OK()*, we see zero panics from the 0.3 crate. In any case, we close this advisory as fixed in 0.3.

…

On Mon, Jan 16, 2023 at 2:18 PM Atul Khare ***@***.***> wrote: Vincent, @vincenthouyi/elf_rs ***@***.***> Thanks a lot for looking into the issue. With version 0.3, we see a ~70% reduction in the total number of crashes, and the remaining 30% appear to be panics that resulted from invalid ELF headers. As I see it, it's perfectly OK to panic when processing invalid input rather than risk continuing program execution. Some implementations may choose to return a Result<> to the caller, but it's an API choice made by the designer of the library. @pinkforest, Please feel free to mark this issue as resolved. Thanks!!! On Sun, Jan 15, 2023 at 7:23 PM Vincent Hou ***@***.***> wrote: > Hi @pinkforest <https://github.com/pinkforest> @atulkharerivos > <https://github.com/atulkharerivos> , I just pushed a new version in > 3ec9935 > <3ec9935> > to solve crashes I found in fuzz test. Please take a look and tell me if > any further crashes you find. > > — > Reply to this email directly, view it on GitHub > <#11 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AZ2XZGACL2466ZFA6RVPBTTWSS5LRANCNFSM6AAAAAARTTFFAU> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >

[vincenthouyi]

@atulkharerivos @pinkforest Thank you very much for hardening the quality of the lib!

Closing the issue as fixed.