From e764a2d16958458e1f431628363f96cfb779f6a4 Mon Sep 17 00:00:00 2001
From: Timo von Hartz <c0untlizzi@gmail.com>
Date: Sat, 22 Feb 2025 07:39:24 +0100
Subject: [PATCH] read_ref: allow zero size reads at any offset (#758)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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`.
---
 src/read/read_ref.rs |  4 ++++
 testfiles            |  2 +-
 tests/read/elf.rs    | 15 +++++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/read/read_ref.rs b/src/read/read_ref.rs
index fb77f011..c348faec 100644
--- a/src/read/read_ref.rs
+++ b/src/read/read_ref.rs
@@ -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(())
diff --git a/testfiles b/testfiles
index 35924ae1..b79486a3 160000
--- a/testfiles
+++ b/testfiles
@@ -1 +1 @@
-Subproject commit 35924ae16ed2a260b1b84932b250cc94fae1f253
+Subproject commit b79486a334e3a5e7fe34a6b73f4fd917c68c6ebe
diff --git a/tests/read/elf.rs b/tests/read/elf.rs
index e42cd516..44261f7f 100644
--- a/tests/read/elf.rs
+++ b/tests/read/elf.rs
@@ -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);
+}