Fix corrupted file_alignment #332
Conversation
…o_t, mapped files
|
@koutheir does this look ok to you? |
|
Thank you for your efforts, @anfedotoff |
|
Some notes:
So I'm wondering why couldn't all the places find_offset was used with the new Result signature not have used the function find_offset_or or left it with
Anyway, I think it's probably fine if this ends up being better maintainable code, but it is a breaking change, so it'll get merged in a little later (along with the other breaking change that's been pending). On that note, may be a good time to think of any other breaking changes for PE stuff that would be good to get in (and it's fine if there are none!) |
src/pe/utils.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn find_offset_or( |
There was a problem hiding this comment.
just fyi: this function existed precisely to wrap the Option returning pub function so that callees that want an error could call this instead (like all the changes in export.rs, for example) without causing breaking changes by altering type signature of find_offset.
The changes are breaking now, however. I guess it's fine since we have another breaking change though.
src/pe/import.rs
Outdated
| &mut utils::find_offset(import_directory_table_rva, sections, file_alignment, opts) | ||
| .ok_or_else(|| { | ||
| error::Error::Malformed(format!( | ||
| "Cannot create ImportData; cannot map import_directory_table_rva {:#x} into offset", |
There was a problem hiding this comment.
we've lost more specific error messages with the new find_offset generic error, although I'm not sure it matters much.
src/pe/exception.rs
Outdated
| let offset = | ||
| utils::find_offset(rva, sections, self.file_alignment, opts).ok_or_else(|| { | ||
| error::Error::Malformed(format!( | ||
| "cannot map exception rva ({:#x}) into offset", |
There was a problem hiding this comment.
ditto all these seem to have lost more specific error messages
|
@anfedotoff i'd be ok with merging this but i think we have to roll back the breaking change to the Thanks for your patience! |
Sorry, for late reply. It's ok if you don't want breaking changes. If we don't want to use |
|
@anfedotoff Ok that works for me! |
This is useful and convenient for assertions.
* Add missing subtraction according to the referenced post * Added detailed explanation for section_read_size function * Breaking change: make Export.offset optional
* complete E_MACHINE constants * complete E_TYPE constants * defined SHF_EXCLUDE
because all earlier ones are not used anyway.
* Guard every with_capacity call with a bounds check The crate does a lot of pre-allocation based on untrusted input, which can lead to unbounded allocation. * Introduce a new Error Variant for Buffer bounds checks
* avoid arithmetic overflow and panics * avoid slice panic in load cmd * avoid underflow in symbol table parsing * validate export trie load command * Warn on invalid offsets
…nings in various feature flag builds
* Fixes missing exports. * Add `new_impl` to `ExportTrie`
* Expose more of the `ProgramHeader` implementation regardless of whether the `alloc` feature is enabled. For example, `ProgramHeader::from_raw_parts` is a wrapper around `core::slice::from_raw_parts` that has `alloc` dependency. * Run `cargo fmt`. Based on an internal patch by Keith Wesolowski (@wesolows). Signed-off-by: Dan Cross <[email protected]>
The well intentioned check on debug directory size unfortunately results in an error when debug information is embedded in the image. Tools like Digital Mars embed CV debug info in the image instead of creating a PDB, and some older tools embed COFF debug info in the image.
* Fix issue #309 - Advance offset by string table field size There's a bug in master where the code ends up misreading the symbol table because we don't advance the offset prior to reading the strings. This change fixes the issue by adding the correct value to the offset and also includes a unit test that covers this case.
This happens when the number of sections in an ELF object reaches 2^16, at which point the u16 shstrndx takes the special value SHN_XINDEX. Fix based on algorithm from: github.com/gimli-rs/object/blob/c476071/src/read/elf/file.rs#L582-L600
fixes #318) * Adjust coff symbol offset to account for the strtable length field
There is little point in using non-loadable segments for the VM address translation. Ignore non-loadable segments.
Multi-arch containers can be made up of archives or Mach-O binaries. This adds support for archives. It is a breaking change because previously the `MachO` struct was returned and now we're returning a new enum: `SingleArch`. This required some refactoring of the `lib.rs` file to share the required functions and data structures for parsing the hint at the top of files. `take_hint_bytes`, `Hint` and `HintData` dont't require any special features but I think were inside the `if_everything!` because that's the only case they were used in. I did expand the API by making `take_byte_hints` public, this was mainly because I thought it was better solution than trying to maintain the various combinations of features required to stop the compiler warning about the function being unused. It's also a function that may be useful for goblin users. Also add tests for parsing fat binaries. This tests parsing fat binaries made up of Mach-O binaries and archives. I've checked in the binaries to make testing easier as they're quite small (both are built from the hello_world.c file in the same directory). Above the tests themselves are instructions for how to compile the binaries they use.
I've done something strange, playing with git history and doing rebase master. I'll better reopen new PR from fresh master. Fix will be very short, sorry) |
Fix for #331