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

Add optional error handling #12

Closed
thenhnn opened this issue May 3, 2024 · 12 comments · Fixed by #13 or #17
Closed

Add optional error handling #12

thenhnn opened this issue May 3, 2024 · 12 comments · Fixed by #13 or #17

Comments

@thenhnn
Copy link

thenhnn commented May 3, 2024

Currently tar_no_std panics when reads malformed data. (I tried fuzzing it and I found a bunch of different errors)
It is not quite good when we have some untrusted source of data (for example, network).

@phip1611
Copy link
Owner

phip1611 commented May 3, 2024

Hi there

I tried fuzzing it and I found a bunch of different errors

awesome, thanks! May I ask how it comes that you are fuzzing my library?

Can you be more specific at which parts the panics occur? This is not a
codebase I frequently work with.

@thenhnn
Copy link
Author

thenhnn commented May 3, 2024

awesome, thanks! May I ask how it comes that you are fuzzing my library?

I'm writing bootloader for my research security-oriented OS project. I thought it would be a great idea to store kernel and initramfs in tar archive and just append signature for verification on bottom, so I just picked this library. My bootloader implements fancy error handling, and I wanted to display if something with tar_no_std is wrong.

Initially I thought that tar_no_std will just return None on next(), but it panicked. I decided to fuzz it.

Can you be more specific at which parts the panics occur? This is not a codebase I frequently work with.

thread '<unnamed>' panicked at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tar-no-std-0.2.0/src/archive.rs:146:9:
assertion `left == right` failed: data must be a multiple of BLOCKSIZE=512
  left: 1
 right: 0
Failing input:

        fuzz/artifacts/tar/crash-adc83b19e793491b1c6ea0fd8b46cd9f32e592fc

Output of `std::fmt::Debug`:

        [10]

@phip1611
Copy link
Owner

phip1611 commented May 3, 2024

I'm writing bootloader for my research security-oriented OS project. I thought it would be a great idea to store kernel and initramfs in tar archive and just append signature for verification on bottom, so I just picked this library

Very similar to my original use case I had when I created this repo. Cool!

@thenhnn
Copy link
Author

thenhnn commented May 3, 2024

@phip1611 I ran AFL for a minute and it found 31 unique panics (from 50k lauches).

This is test case:

let archive = TarArchiveRef::new(data);
match archive {
  Ok(entries) => println!("{:#?}", entries.entries().collect::<Vec<_>>()),
  Err(err) => println!("{:#?}", err),
}

These files cause tar_no_std to panic (I used latest dev2 revision):
crashes.tar.gz

@phip1611
Copy link
Owner

phip1611 commented May 3, 2024

I think the latest version on main is fine now. Can you confirm?

@thenhnn
Copy link
Author

thenhnn commented May 3, 2024

I think the latest version on main is fine now. Can you confirm?

It's not.

@thenhnn
Copy link
Author

thenhnn commented May 3, 2024

@phip1611 I ran AFL for a minute and it found 31 unique panics (from 50k lauches).

This is test case:

let archive = TarArchiveRef::new(data);
match archive {
  Ok(entries) => println!("{:#?}", entries.entries().collect::<Vec<_>>()),
  Err(err) => println!("{:#?}", err),
}

These files cause tar_no_std to panic (I used latest dev2 revision): crashes.tar.gz

Can you check these inputs?

@phip1611
Copy link
Owner

phip1611 commented May 3, 2024

I ran

#[test]
    fn test_foo() {
        let data =include_bytes!("./crashes.tar");
        let archive = TarArchiveRef::new(data).unwrap();
        let x = archive.entries().collect::<Vec<_>>();
        dbg!(x);
    }

with the following output

[
    ArchiveEntry {
        filename: "./id:000000,sig:06,src:000008,time:65,execs:3494,op:colorization,pos:0",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000001,sig:06,src:000008,time:66,execs:3512,op:colorization,pos:0",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000002,sig:06,src:000008,time:70,execs:3626,op:colorization,pos:0",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000003,sig:06,src:000008,time:70,execs:3627,op:colorization,pos:0",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000004,sig:06,src:000008,time:75,execs:3752,op:colorization,pos:0",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000005,sig:06,src:000008,time:132,execs:5160,op:colorization,pos:0",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000006,sig:06,src:000008,time:203,execs:6696,op:colorization,pos:0",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000007,sig:06,src:000008,time:374,execs:10649,op:havoc,rep:5",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000008,sig:06,src:000008,time:375,execs:10701,op:havoc,rep:6",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000009,sig:06,src:000008,time:378,execs:10805,op:havoc,rep:3",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000010,sig:06,src:000008,time:379,execs:10829,op:havoc,rep:5",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000011,sig:06,src:000008,time:383,execs:10967,op:havoc,rep:6",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000012,sig:06,src:000008,time:386,execs:11062,op:havoc,rep:3",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000013,sig:06,src:000008,time:389,execs:11152,op:havoc,rep:8",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000014,sig:06,src:000008,time:391,execs:11238,op:havoc,rep:7",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000015,sig:06,src:000008,time:395,execs:11372,op:havoc,rep:6",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000016,sig:06,src:000008,time:397,execs:11453,op:havoc,rep:6",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000017,sig:06,src:000008,time:401,execs:11607,op:havoc,rep:2",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000018,sig:06,src:000008,time:403,execs:11697,op:havoc,rep:3",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000019,sig:06,src:000008,time:405,execs:11734,op:havoc,rep:3",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000020,sig:06,src:000008,time:450,execs:13284,op:havoc,rep:8",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000021,sig:06,src:000008,time:473,execs:14106,op:havoc,rep:6",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000022,sig:06,src:000008,time:479,execs:14310,op:havoc,rep:4",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000023,sig:06,src:000008,time:527,execs:16172,op:havoc,rep:5",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000024,sig:06,src:000008,time:532,execs:16369,op:havoc,rep:4",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000025,sig:06,src:000008,time:544,execs:16820,op:havoc,rep:8",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000026,sig:06,src:000008+000007,time:638,execs:18748,op:splice,rep:4",
        size: 10240,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000027,sig:06,src:000008+000007,time:654,execs:18965,op:splice,rep:2",
        size: 10240,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000028,sig:06,src:000036,time:1258,execs:38836,op:colorization,rep:1",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000029,sig:06,src:000033+000043,time:1484,execs:46275,op:splice,rep:1",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000030,sig:06,src:000060,time:2062,execs:68617,op:havoc,rep:2",
        size: 3584,
        data: "<bytes>",
    },
    ArchiveEntry {
        filename: "./id:000031,sig:06,src:000103,time:61195,execs:2077165,op:havoc,rep:2",
        size: 11776,
        data: "<bytes>",
    },
]

So I'm not sure where the problem is?

@thenhnn
Copy link
Author

thenhnn commented May 3, 2024

I ran

#[test]
    fn test_foo() {
        let data =include_bytes!("./crashes.tar");
        let archive = TarArchiveRef::new(data).unwrap();
        let x = archive.entries().collect::<Vec<_>>();
        dbg!(x);
    }

with the following output

[
   ...
]

So I'm not sure where the problem is?

This is just archive that contains files that trigger panic. Try to unpack it and then read from, for example include_bytes!("./cases/id:000000,sig:06,src:000008,time:65,execs:3494,op:colorization,pos:0").

@phip1611
Copy link
Owner

phip1611 commented May 3, 2024

Ah, got it. Sorry

@thenhnn
Copy link
Author

thenhnn commented May 3, 2024

@phip1611 Can you reopen?

@phip1611
Copy link
Owner

phip1611 commented May 3, 2024

Thanks very much. Your tarball collection now runs in CI. #17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants