Skip to content

Commit

Permalink
read_ref: allow zero size reads at any offset (#758)
Browse files Browse the repository at this point in the history
Some ELF files do have segments with size zero, at offsets that are
invalid / out of bounds. This commit changes the default `ReadRef`
implementation for `&[u8]` to permit reads that are out of bounds, if
the requested length is zero, by returning `&[]`.

Examples of such files are debug files created with `objcopy --only-keep-debug`,
for example

`/usr/lib/debug/.build-id/bd/dd2eaf3326ffce6d173666b5f3e62a376e123a.debug`
in package `libgedit-gfls-1-0-dbgsym_0.2.1-2_arm64.deb`:

```
~ cp /usr/lib/debug/.build-id/bd/dd2eaf3326ffce6d173666b5f3e62a376e123a.debug /tmp/bad_file.elf
~ r2 /tmp/bad_file.elf
[0x0000027c]> iI~binsz
binsz    64184
[0x0000027c]> iSS
[Segments]

nth paddr        size vaddr        vsize perm type name
―――――――――――――――――――――――――――――――――――――――――――――――――――――――
0   0x00000000  0x27c 0x00000000  0x55e0 -r-x MAP  LOAD0
1   0x0000fab8    0x0 0x0001fab8   0x5c0 -rw- MAP  LOAD1
2   0x0000fab8    0x0 0x0001fb28   0x240 -rw- MAP  DYNAMIC
[...]
8   0x0000fab8    0x0 0x0001fab8   0x548 -r-- MAP  GNU_RELRO
```

This file has multiple segments starting at `0xfab8` (the end of the
file), with a physical size of `0`, while the file size is also `0xfab8`
(64184).

The current `ReadRef` implementation for `&[u8]` causes
`ProgramHeader::data` to fail for them, causing e.g. `ElfSegment::bytes`
to also fail.

While a caller could handle this error, or one could provide their own
type implementing `ReadRef` this way, I think it makes sense to do this
by default, since no data is *actually* being read out of bounds, but
with the current implementation we still try to index out of bounds and
then error.

Notably with this change this also matches the implementation of
`ReadRef` for `ReadCache` which already has a similar check implemented
for `read_bytes_at`.
  • Loading branch information
th0rex authored Feb 22, 2025
1 parent a27551b commit e764a2d
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
4 changes: 4 additions & 0 deletions src/read/read_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ impl<'a> ReadRef<'a> for &'a [u8] {
}

fn read_bytes_at(self, offset: u64, size: u64) -> Result<&'a [u8]> {
if size == 0 {
return Ok(&[]);
}

let offset: usize = offset.try_into().map_err(|_| ())?;
let size: usize = size.try_into().map_err(|_| ())?;
self.get(offset..).ok_or(())?.get(..size).ok_or(())
Expand Down
2 changes: 1 addition & 1 deletion testfiles
15 changes: 15 additions & 0 deletions tests/read/elf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,18 @@ fn get_buildid_less_bad_elf() {
b"\xf9\xc0\xc6\x05\xd3\x76\xbb\xa5\x7e\x02\xf5\x74\x50\x9d\x16\xcc\xe9\x9c\x1b\xf1"
);
}

#[cfg(feature = "std")]
#[test]
fn zero_sized_section_works() {
use object::{Object as _, ObjectSection as _};
let path: PathBuf = ["testfiles", "elf", "base.debug"].iter().collect();
let data = std::fs::read(&path).unwrap();
let object = object::read::File::parse(&data[..]).unwrap();

// The unwrap here should not fail, even though the section has an invalid offset, its size is
// zero so this should succeed.
let section = object.section_by_name(".bss").unwrap();
let data = section.data().unwrap();
assert_eq!(data.len(), 0);
}

0 comments on commit e764a2d

Please # to comment.