It would be extremely helpful to use the capabilities of goblin when working with memory/process dumps as well as files on disk. Since memory dumps are already mapped we can just skip over the rva resolve function and all of the other functionality will work still.

I'm not sure however if it would be best to implement this as a compile-time feature (as this PR does it) or as a runtime feature (maybe splitting the parse function into parse_image / parse_mapped or something similiar.

Wrapped in a separate tiny crate this time, because it's just that bit of functionality that totally should be in core. It is possible to go one step further and implement #[derive(Plain)], with static checking of the prescribed requirements, but I'm not yet guru enough to do that.

I was investigating my options for parsing EXE files to determine what environment to auto-fill in my experimental game launcher (ie. DOSBox, Wine, Wine+qemu-user, Mono, etc.) and I managed to trigger some panics in goblin.

====================
TESTING WITH GOBLIN:
====================
unknown magic: ./hello_owatcom_com.com
Parse error: ./hello_owatcom_os2v2.exe => Invalid magic number: 0x1
pe: ./hello_pacific.exe
Parse error: ./hello_owatcom_dos.upx.exe => requested range [309100590..309100594) from object of len 6881
Parse error: ./hello_owatcom_dos4g.exe => Invalid magic number: 0x1
Parse error: ./hello_owatcom_windows.exe => Invalid magic number: 0x0
pe: ./hello_mingw32.exe
pe: ./hello_csharp_exe_itanium.exe
pe: ./hello_owatcom_win95.exe
Parse error: ./hello_owatcom_dos4g.upx.exe => Invalid magic number: 0x1
elf: ./hello_gcc.x86
unknown magic: ./hello_djgpp.upx.coff.exe
unknown magic: ./hello_owatcom_com.upx.com
unknown magic: ./hello_dev86.upx.com
unknown magic: ./hello_dev86.com
pe: ./hello_mingw64.exe
Parse error: ./hello_djgpp.exe => Invalid magic number: 0x0
PANICKED on hello_mingw32.upx.exe
Parse error: ./hello_owatcom_dos.exe => Invalid magic number: 0x20
PANICKED on hello_owatcom_win95.upx.exe
pe: ./hello_owatcom_nt.exe
Parse error: ./hello_owatcom_win386.exe => Invalid magic number: 0x0
Parse error: ./hello_djgpp.upx.exe => Invalid magic number: 0x0
Parse error: ./hello_owatcom_dos4gnz.exe => Invalid magic number: 0x1
PANICKED on hello_mingw64.upx.exe
Parse error: ./hello_owatcom_os2.exe => Invalid magic number: 0x0
pe: ./hello_csharp_exe_arm.exe
pe: ./hello_csharp_exe_x64.exe
pe: ./hello_csharp_exe_x86.exe
elf: ./hello_gcc.x86_64
PANICKED on hello_owatcom_nt.upx.exe
Parse error: ./hello_pacific.upx.exe => requested range [309100590..309100594) from object of len 4527

Here's a backtrace which appears to represent all of the panics:

ssokolow@monolith test_exes [rusty-core] % RUST_BACKTRACE=1 ./pe-test/target/debug/goblin-test hello_owatcom_nt.upx.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:329
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:355
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:371
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:549
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:511
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:495
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:471
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:69
   9: core::panicking::panic
             at /checkout/src/libcore/panicking.rs:49
  10: <core::option::Option<T>>::unwrap
             at /checkout/src/libcore/macros.rs:21
  11: goblin::pe::import::SyntheticImportDirectoryEntry::parse
             at /home/ssokolow/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.0.10/src/pe/import.rs:125
  12: goblin::pe::import::ImportData::parse
             at /home/ssokolow/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.0.10/src/pe/import.rs:158
  13: goblin::pe::PE::parse
             at /home/ssokolow/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.0.10/src/pe/mod.rs:80
  14: goblin::parse
             at /home/ssokolow/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.0.10/src/lib.rs:276
  15: goblin_test::run
             at ./pe-test/src/goblin.rs:17
  16: goblin_test::main
             at ./pe-test/src/goblin.rs:36
  17: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  18: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:433
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:57
  19: main
  20: __libc_start_main
  21: <unknown>

While this renders it unsuitable for my project (the mere fact that Goblin is capable of dying at an unwrap (when the other PE parser I've tried so far simply used Result to indicate a parse failure) indicates that using it in my project would cause me more worry than simply writing my own MZ/NE/PE parser with Nom), I thought you'd want to know so you can fix the problem for others.

If you want to re-create my test binaries, the source materials are in the test_exes folder of ssokolow/game_launcher and build.sh contains instructions for the simplest, easiest way to install the requisite packages on a *buntu Linux 14.04 LTS machine like mine.

To reiterate what build.sh says, all compilers are optional, so producing just the binaries which caused panics here should only require apt-get install upx-ucl mingw-w64 and then downloading and unpacking OpenWatcom.

Fixes the resolution of redirected unwind information in the PE exception table.

The issue originated in https://github.com/m4b/goblin/pull/136. Runtime function entries usually specify an offset of a UnwindInfo struct, which is always 4-byte aligned. However, sometimes the last bit of this offset is flipped, with indicates that instead another RuntimeFunction must be resolved at the offset (with the bit flipped back). This logic is handled by get_unwind_info.

There was a bug in the initial implementation, that treated the offset as relative, where in fact it is absolute. To fix this, get_function_by_offset now expects an absolute offset.

example

$ cat src/main.rs
use std::fs::File;
use std::io::Read;
use std::env;
use goblin::elf::*;

fn main() -> Result<(), Box<dyn std::error::Error>> {
  let args = env::args().collect::<Vec<String>>();
  if args.len() < 2 {
      eprintln!("Usage: {} binary", args[0])
  } else {
      let mut f = File::open(&args[1])?;
      let mut buf = Vec::new();
      f.read_to_end(&mut buf)?;
      let elf = Elf::parse(&buf)?;
      let strtab = elf.shdr_strtab.to_vec().unwrap();
      for s in strtab {
          println!("{}", s)
      }
  }

  Ok(())
}
$ cargo r `which ls`
~ snip ~
    Finished dev [unoptimized + debuginfo] target(s) in 0.16s
     Running `target/debug/readstrtab /usr/bin/ls`

.shstrtab
.interp
.note.gnu.property
.note.gnu.build-id
.note.ABI-tag
.gnu.hash
.dynsym
.dynstr
.gnu.version
.gnu.version_r
.rela.dyn
.rela.plt
.init
.plt.got
.plt.sec
.text
.fini
.rodata
.eh_frame_hdr
.eh_frame
.init_array
.fini_array
.data.rel.ro
.dynamic
.data
.bss
.gnu_debugaltlink
.gnu_debuglink
$ readelf -S `which ls`|grep "\["
  [Nr] Name              Type             Address           Offset
  [ 0]                   NULL             0000000000000000  00000000
  [ 1] .interp           PROGBITS         0000000000000318  00000318
  [ 2] .note.gnu.pr[...] NOTE             0000000000000338  00000338
  [ 3] .note.gnu.bu[...] NOTE             0000000000000358  00000358
  [ 4] .note.ABI-tag     NOTE             000000000000037c  0000037c
  [ 5] .gnu.hash         GNU_HASH         00000000000003a0  000003a0
  [ 6] .dynsym           DYNSYM           0000000000000450  00000450
  [ 7] .dynstr           STRTAB           0000000000001050  00001050
  [ 8] .gnu.version      VERSYM           0000000000001616  00001616
  [ 9] .gnu.version_r    VERNEED          0000000000001718  00001718
  [10] .rela.dyn         RELA             00000000000017b8  000017b8
  [11] .rela.plt         RELA             0000000000002bf8  00002bf8
  [12] .init             PROGBITS         0000000000004000  00004000
  [13] .plt              PROGBITS         0000000000004020  00004020
  [14] .plt.got          PROGBITS         00000000000046c0  000046c0
  [15] .plt.sec          PROGBITS         00000000000046f0  000046f0
  [16] .text             PROGBITS         0000000000004d80  00004d80
  [17] .fini             PROGBITS         0000000000018bc4  00018bc4
  [18] .rodata           PROGBITS         0000000000019000  00019000
  [19] .eh_frame_hdr     PROGBITS         000000000001e324  0001e324
  [20] .eh_frame         PROGBITS         000000000001ec70  0001ec70
  [21] .init_array       INIT_ARRAY       0000000000022fd0  00021fd0
  [22] .fini_array       FINI_ARRAY       0000000000022fd8  00021fd8
  [23] .data.rel.ro      PROGBITS         0000000000022fe0  00021fe0
  [24] .dynamic          DYNAMIC          0000000000023a58  00022a58
  [25] .got              PROGBITS         0000000000023c58  00022c58
  [26] .data             PROGBITS         0000000000024000  00023000
  [27] .bss              NOBITS           0000000000024280  00023268
  [28] .gnu_debugaltlink PROGBITS         0000000000000000  00023268
  [29] .gnu_debuglink    PROGBITS         0000000000000000  000232b4
  [30] .shstrtab         STRTAB           0000000000000000  000232e8

.pltdoesn't appear in shdr_strtab.

GNU symbol version extension for ELF files (#263).

WIP PR to discuss overall design.

Tasks:

• [x] Parse .gnu.version_r section
• [x] Parse .gnu.version_d section
• [x] Parse .gnu.version section
• [x] Test parsing .gnu.version_r section
• [x] Test parsing .gnu.version_d section
• [x] Test parsing .gnu.version section

(This is currently a WIP PR, for other members of safety-dance to help audit and improve the code)

This PR aims to tackle the issues raised in https://github.com/rust-secure-code/safety-dance/issues/8 :

• unsafe { ... } blocks and unsafe impls had no

  // # Safety
  //
  //   - ...
  

  annotations, which makes it easy to forget or miss safety requirements / invariants to uphold, be it when the code is written, or later when the code is modified; it also makes quickly auditing the code harder. That's why, imho, it is very important to have such safety comment annotations;

• The usages of unsafe corresponded to four cases:

  • .get_unchecked indexing for pe/data_directories array getters.

    • These have been factored in a macro to avoid code repetition, and within that macro compile-time assertions have been added so that the code is robust to changes;

  • unsafe fn exported to the API;

    • These have not been audited (yet): by virtue of being marked unsafe, users of the library must explicitely opt into calling these unsafe functions, and it is thus (mainly) their responsibility to do it correctly, hence making it a low-priority fix.

  • unsafe impl (for ::plain::Plain).

    • These (implicitly) relied on each field also being Plain, such as integers, but where also used with a macro (à la C++ template): hence a static assertion has been added in those cases to ensure that the "template" type parameter is indeed Plain.

    • Plain on its own offers non-unsafe transmute-based APIs to go into and from slices of bytes; the soundness responsibility of it falls down on the ::plain crate and has not been audited either. Ideally, all the ::plain usages could be replaced by ::zerocopy, since thanks to the #[derive(...)] it offers compile-time checks for the soundness of these impls.

  • unsafe { fd.read_exact(plain::as_mut_bytes(&mut /* some structure */)?); }

    • These have been the main change of the PR: with the compile-time checked #[derive(AsBytes)], we get access to a non-unsafe ::zerocopy::AsBytes::as_bytes_mut for equivalent functionality.

    • This, in turn, has showed that there were some structures that did have padding, which has been removed with the #[repr(C, packed)] annotation, and the appropriate ptr::read_unaligned-based getters.

Fixes #99

An early PR to get some reviews, as there are probably better ways to do this. I need to implement the base64 kind of section table names, and fix the error handling in some places.

Adds support for archives entries in a fat binary. I ended up going down the breaking change route because I wasn't sure of a good way of doing it without one. Super happy to do another pass on this if you think there is a way!

Basically, there's just an enum MultiArchEntry with variants for a MachO or an Archive. All the functions for retrieving entries in MultiArch now return this enum, rather than MachO. This does mean you could technically represent a fat binary with a mixture of binaries and archives even though I'm not sure if that is actually allowed.

Testing-wise, I've added tests for parsing fat binaries (both the binary and archive variants). They require binaries to be checked in which I've done. I thought it was easiest to check them in as they're pretty small and I saw there was already a DLL file checked in. Alternatively, we could build them as part of the test if you'd prefer, though that would mean that the tests could only run on MacOS because I don't think lipo exists on Linux.

This implements parsing for the exception tables data directory and x64 unwind codes. Respective documentation from Microsoft is located here: https://docs.microsoft.com/en-us/cpp/build/exception-handling-x64?view=vs-2017.

The exception directory is only present on x64 builds, so the entire implementation can assume x64 exception handling.

Tests are still missing as I haven't taken the time to extract relevant bits from an actual executable for realistic testing yet.

We sort the package section based on https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/cargo.md

@m4b Do we need to include examples, tests and fuzz directories in goblin crate?

MS doc: https://learn.microsoft.com/en-us/windows/win32/menurc/string-str?redirectedfrom=MSDN Yara rule support for field: https://yara.readthedocs.io/en/v3.2.0/modules/pe.html

This is a useful field in threat hunting and forensics in general.

thanks

overflow/underflow issue with this line of code it seems:

https://docs.rs/goblin/latest/src/goblin/pe/symbol.rs.html#240

stack backtrace:
   0: rust_begin_unwind
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/panicking.rs:65:14
   2: core::panicking::panic
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/panicking.rs:115:5
   3: goblin::pe::symbol::Symbol::name_offset::{{closure}}
             at /home/username/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.6.0/src/pe/symbol.rs:240:36
   4: core::option::Option<T>::map
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/option.rs:925:29
   5: goblin::pe::symbol::Symbol::name_offset

While trimming down my dependencies, I noticed that building Goblin with default-features = false, features = ["std", "elf32"] fails:

    Checking goblin v0.6.0
error[E0432]: unresolved import `crate::elf64`
   --> /Users/mjk/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.6.0/src/elf/header.rs:249:17
    |
249 |             use crate::elf64;
    |                 ^^^^^^^^^^^^ no `elf64` in the root

error[E0433]: failed to resolve: could not find `elf64` in the crate root
   --> /Users/mjk/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.6.0/src/elf/dynamic.rs:802:35
    |
802 |     elf_dyn_std_impl!(u64, crate::elf64::program_header::ProgramHeader);
    |                                   ^^^^^ could not find `elf64` in the crate root

Some errors have detailed explanations: E0432, E0433.
For more information about an error, try `rustc --explain E0432`.

This isn't a big deal – I can add elf64 to the feature list – but the Rust Book advises against it:

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features.

There is an extension to gABI that add this program header : see https://raw.githubusercontent.com/wiki/hjl-tools/linux-abi/linux-abi-draft.pdf

Would it be possible to consider adding it in?

The relevant sections :

3.1 Program header
The following Linux program header types are defined:
Table 3.1: Program Header Types
Name Value
PT_GNU_EH_FRAME 0x6474e550
PT_GNU_PROPERTY 0x6474e553
PT_GNU_EH_FRAME The segment contains .eh_frame_hdr section. See
Section 2.1.3 of this document.
PT_GNU_PROPERTY The segment contains .note.gnu.property sec-
tion. See Section 2.1.5 of this document.

2.1.5 .note.gnu.property section
.note.gnu.property section contains a program property note which de-
scribes special handling requirements for linker and run-time loader. It can be
merged with other SHT_NOTE sections.
Table 2.9: The Program Property Note Format
Field Length Contents
n_namsz 4 4
n_descsz 4 The note descriptor size
n_type 4 NT_GNU_PROPERTY_TYPE_0
n_name 4 GNU
n_desc n_descsz The program property array
n_namsz Size of the n_name field. A 4-byte integer in the format of the target
processor. It should be 4.
n_descsz Size of the n_desc field. A 4-byte integer in the format of the target
processor.
n_type Type of the note descriptor. A 4-byte integer in the format of the target
processor. It should be NT_GNU_PROPERTY_TYPE_0.
n_name Owner of the program property note. A null-terminated character string.
It should be GNU.
n_desc The note descriptor. The first n_descsz bytes in n_desc is the pro-
gram property array.

There is no way to get access to null-terminated C strings that are not valid UTF-8, through goblin::strtab::Strtab. Strtab.get() and Strtab.get_at() should return Option<&CStr> instead of optional UTF-8 strs.

In ELF, the DT_JMPREL dynamic array tag is optional, and can be present (holding an address value) or absent. The value of that tag is exposed as goblin::elf::dynamic::DynamicInfo.jmprel, which is of type usize. DynamicInfo.jmprel is set to zero if DT_JMPREL is absent from the ELF file.

Therefore, based on the definition of goblin::elf::dynamic::DynamicInfo, there is no way to differentiate the absence of DT_JMPREL from its presence with the value zero, because zero can be a valid address, especially in privileged mode and in embedded systems.

I think DT_JMPREL should be exposed by DynamicInfo as optional information. We should make the type of DynamicInfo.jmprel an Option<usize> instead.