Crash on malformed ELF file

[Shnatsel]

Attempting to decode any of the attached files with goblin::elf::Elf::parse crashes the process. Memory allocator runs out of virtual memory and the process is aborted.

goblin-elf-oom-crashes.zip

Found via AFL.rs. Fuzzing harness: https://github.com/Shnatsel/goblin/blob/master/fuzz-afl/src/main.rs

[m4b]

Many parts of the parser rely on the binary giving lengths to various variable length entities; I guess the best we can do here is to clamp the maximum size to the size of the binary?

[Shnatsel]

Sounds like a great idea!

Alternatively you could let the API user set the limits, which is what the decompression libraries do. But if you know the size up front, it's probably best to just use that.

[m4b]

@Shnatsel do you want to make a PR :)

[Shnatsel]

It would be interesting, but I'm afraid I'll have to decline. I already have two ongoing Rust projects that have higher priority, and one of them isn't even announced yet.

[philipc]

In general, how does goblin want to handle errors: parse as much as it can, or return an error immediately when something is obviously wrong? For example, if the size of a section is larger than the size of the file, should it still try to parse whatever it can in that section? Or should it skip that section but keep trying to parse other sections? Or should it immediately return an error?

I can have a look at some of these. However, not all of them are giving errors for me.
Running:

for i in all-oom-crashes/*; do target/debug/examples/rdr $i 2&>1 >/dev/null && echo $i; done

gives:

all-oom-crashes/id:000000,sig:06,src:000001,op:flip1,pos:135
all-oom-crashes/id:000000,sig:06,src:000001,op:havoc,rep:32
all-oom-crashes/id:000000,sig:06,src:000001,op:havoc,rep:4
all-oom-crashes/id:000000,sig:06,src:000001,op:havoc,rep:64
all-oom-crashes/id:000001,sig:06,src:000001,op:flip1,pos:135
all-oom-crashes/id:000001,sig:06,src:000001,op:havoc,rep:128
all-oom-crashes/id:000001,sig:06,src:000001,op:havoc,rep:32
all-oom-crashes/id:000001,sig:06,src:000001,op:havoc,rep:64
all-oom-crashes/id:000001,sig:06,src:000016+000133,op:splice,rep:128
all-oom-crashes/id:000001,sig:06,src:000027,op:havoc,rep:16
all-oom-crashes/id:000001,sig:06,src:000094+000140,op:splice,rep:64
all-oom-crashes/id:000002,sig:06,src:000001+000048,op:splice,rep:128
all-oom-crashes/id:000002,sig:06,src:000001,op:havoc,rep:32
all-oom-crashes/id:000002,sig:06,src:000029+000342,op:splice,rep:64
all-oom-crashes/id:000002,sig:06,src:000031,op:havoc,rep:16
all-oom-crashes/id:000002,sig:06,src:000107+000371,op:splice,rep:128
all-oom-crashes/id:000002,sig:06,src:000123+000378,op:splice,rep:64
all-oom-crashes/id:000003,sig:06,src:000001,op:havoc,rep:128
all-oom-crashes/id:000003,sig:06,src:000009+000190,op:splice,rep:128
all-oom-crashes/id:000003,sig:06,src:000088+000299,op:splice,rep:128
all-oom-crashes/id:000003,sig:06,src:000107+000371,op:splice,rep:2
all-oom-crashes/id:000004,sig:06,src:000001,op:havoc,rep:128
all-oom-crashes/id:000004,sig:06,src:000006,op:havoc,rep:16
all-oom-crashes/id:000004,sig:06,src:000106,op:havoc,rep:2
all-oom-crashes/id:000004,sig:06,src:000111+000267,op:splice,rep:32
all-oom-crashes/id:000004,sig:06,src:000316+000306,op:splice,rep:128
all-oom-crashes/id:000005,sig:06,src:000001,op:havoc,rep:128
all-oom-crashes/id:000005,sig:06,src:000130,op:havoc,rep:2
all-oom-crashes/id:000005,sig:06,src:000147+000331,op:splice,rep:128
all-oom-crashes/id:000005,sig:06,src:000173,op:havoc,rep:4
all-oom-crashes/id:000006,sig:06,src:000106+000413,op:splice,rep:2
all-oom-crashes/id:000006,sig:06,src:000164+000258,op:splice,rep:32
all-oom-crashes/id:000006,sig:06,src:000359+000301,op:splice,rep:64
all-oom-crashes/id:000006,sig:06,src:000370+000361,op:splice,rep:8
all-oom-crashes/id:000007,sig:06,src:000012+000307,op:splice,rep:16
all-oom-crashes/id:000007,sig:06,src:000039+000227,op:splice,rep:128
all-oom-crashes/id:000007,sig:06,src:000280,op:havoc,rep:64
all-oom-crashes/id:000007,sig:06,src:000373+000330,op:splice,rep:32
all-oom-crashes/id:000007,sig:06,src:000405,op:havoc,rep:128
all-oom-crashes/id:000008,sig:06,src:000019+000207,op:splice,rep:128
all-oom-crashes/id:000008,sig:06,src:000064+000316,op:splice,rep:32
all-oom-crashes/id:000008,sig:06,src:000132+000195,op:splice,rep:64
all-oom-crashes/id:000008,sig:06,src:000362,op:flip1,pos:199
all-oom-crashes/id:000008,sig:06,src:000439+000429,op:splice,rep:128
all-oom-crashes/id:000008,sig:06,src:000445,op:havoc,rep:128
all-oom-crashes/id:000008,sig:06,src:000468,op:havoc,rep:64
all-oom-crashes/id:000009,sig:06,src:000105+000410,op:splice,rep:32
all-oom-crashes/id:000009,sig:06,src:000152+000210,op:splice,rep:128
all-oom-crashes/id:000009,sig:06,src:000162+000255,op:splice,rep:64
all-oom-crashes/id:000009,sig:06,src:000449,op:havoc,rep:64
all-oom-crashes/id:000009,sig:06,src:000462,op:havoc,rep:32
all-oom-crashes/id:000010,sig:06,src:000008+000197,op:splice,rep:64
all-oom-crashes/id:000010,sig:06,src:000057+000278,op:splice,rep:128
all-oom-crashes/id:000010,sig:06,src:000121+000443,op:splice,rep:128
all-oom-crashes/id:000010,sig:06,src:000547,op:havoc,rep:64
all-oom-crashes/id:000011,sig:06,src:000115+000003,op:splice,rep:128
all-oom-crashes/id:000011,sig:06,src:000231,op:havoc,rep:128
all-oom-crashes/id:000011,sig:06,src:000448,op:havoc,rep:64
all-oom-crashes/id:000011,sig:06,src:000641+000403,op:splice,rep:64
all-oom-crashes/id:000012,sig:06,src:000128,op:havoc,rep:8
all-oom-crashes/id:000012,sig:06,src:000133,op:havoc,rep:128
all-oom-crashes/id:000012,sig:06,src:000644+000138,op:splice,rep:32
all-oom-crashes/id:000012,sig:06,src:000649+000129,op:splice,rep:128
all-oom-crashes/id:000013,sig:06,src:000052+000303,op:splice,rep:128
all-oom-crashes/id:000013,sig:06,src:000133,op:havoc,rep:128
all-oom-crashes/id:000013,sig:06,src:000410,op:havoc,rep:64
all-oom-crashes/id:000014,sig:06,src:000281+000336,op:splice,rep:64
all-oom-crashes/id:000014,sig:06,src:000690,op:havoc,rep:32
all-oom-crashes/id:000015,sig:06,src:000690,op:havoc,rep:64
all-oom-crashes/id:000016,sig:06,src:000214+000230,op:splice,rep:128
all-oom-crashes/id:000016,sig:06,src:000339,op:havoc,rep:128
all-oom-crashes/id:000016,sig:06,src:000349+000075,op:splice,rep:64
all-oom-crashes/id:000016,sig:06,src:000641+000349,op:splice,rep:128
all-oom-crashes/id:000019,sig:06,src:000328+000071,op:splice,rep:64
all-oom-crashes/id:000019,sig:06,src:000376+000232,op:splice,rep:8
all-oom-crashes/id:000021,sig:06,src:000366+000186,op:splice,rep:16
all-oom-crashes/id:000021,sig:06,src:000468+000227,op:splice,rep:128

[m4b]

These are all good questions, and I'm not sure what the best answer is; I think we've tried to be as robust as possible in the past, but it starts getting into a grey area; overall however, I've found it almost always better for the parser to just keep going even when stuff is kind of broken, as opposed to just returning immediately and stopping all parsing, since that forces you to dive into a hexdump or some other tool if you want more details and the error message isn't super useful.

[philipc]

For my purposes, I think what I want is for goblin to provide lower level APIs that parse one thing, and return an error if that fails. It's then up to me to decide whether I want to continue trying to parse other things. goblin doesn't currently provide that sort of lower level API though. It tries to parse everything into a struct Elf, and for that API I agree that's it's better to try to keep going, otherwise one little error prevents you from getting anything.

[philipc]

Fixed what I could in #121. There's probably more similar problems. Not something I'm going to work on right now though.

[philipc]

Representative test cases for the problems I fixed:

all-oom-crashes/id:000000,sig:06,src:000001,op:flip1,pos:135
all-oom-crashes/id:000005,sig:06,src:000363,op:havoc,rep:2

[jackcmay]

@philipc Ran into a similar issue with section headers and Vec::with_capacity while fuzzing it myself. Took a stab at fixing it here: #219

[philipc]

Looks reasonable to me, but I'll let @m4b review. Really need to audit every Vec::with_capacity instead of relying on fuzzing though.