PE: ImportLookupTableEntry::parse() panics

[willglynn]

I'm getting panics on this line while trying to parse a particular executable. Unfortunately, this executable is proprietary so I can't share it, and I don't know enough about the PE format to understand what's going on here.

I did however write a script to run through the PE executables on my machine, which found that a random GDAL distribution I had laying around includes a curl.exe that causes the exact same panic. curl is something I can share, so steps to reproduce are:

$ wget -q https://s3.willglynn.com/goblin/curl.exe
$ RUST_BACKTRACE=1 cargo run --example rdr curl.exe 
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/examples/rdr curl.exe`
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:335
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::panicking::panic
   9: <core::option::Option<T>>::unwrap
  10: goblin::pe::import::ImportLookupTableEntry::parse
  11: goblin::pe::import::SyntheticImportDirectoryEntry::parse
  12: goblin::pe::import::ImportData::parse
  13: goblin::pe::PE::parse
  14: goblin::parse
  15: rdr::run
  16: rdr::main
  17: __rust_maybe_catch_panic
  18: std::rt::lang_start
  19: main
$ git rev-parse --short HEAD
1595f19

Come to think of it, I bet my original executable statically links libcurl, so even though these executables are from totally different environments, that might be a common thread.

[m4b]

@willglynn awesome thanks for providing what you could, will make much easier !

I'm not in front of a puter right now but I will look into this asap, pretty sure it was a case of me unwrapping when I shouldn't have, the PE backend, is alas, slightly less filled with love than Mach or ELF.

Also, I assume from the repro this is coming from goblin master? I don't know the commit offhand but if it's from a few days ago I just merged a huge change so could be related to that.

Anyyyyywayy thanks for the repro!

[willglynn]

Great, thanks! 👍

That commit is indeed latest master, though I originally encountered the issue running 0.0.10. I've hacked things around so as to not be stuck, but I have no idea what these PE files are trying to say so I don't dare submit a patch.

I did notice that typical values for rva appear to be largeish offsets within the file, while typical values for ordinal tend to be tiny integers, and failing values for rva in both the public and private executables appear to also be small integers… but yeah, I'm out of my depth there.

(If you want to know about PDB files, on the other hand…)

[m4b]

@willglynn I believe this resolves it at least in the case of curl, though why the binary has that bad RVA is a bit mysterious; i wonder what the kernel is doing when it hits that RVA 🤔

At first I thought it wasn't bad, but I was doing ordinals wrong (since the RVA was small (0xa), which had no chance of being found in sections starting from 0x1000 onwards, I thought it was an ordinal); but I checked and I'm pretty sure I'm correctly parsing ordinals, so not sure what's going on with that import from WS2_32.dll

[willglynn]

Yep! master works on the proprietary executables as well. Thanks!

[philipc]

For reference, this was actually a bug with parsing PE32+ imports, which was later fixed by #82.