Repository: m4b/goblin (language: rust)

Issue:
Unable to parse ELF SectionHeader from raw bytes
I am unsure if this is a side effect of something else or done on purpose but let's imagine the following code:
```
            let mut fs = fs::File::open(path)?;
            let section_header_bytes = copy_array(
                fs,
                elf_head.e_shoff as usize,
                (elf_head.e_shnum as usize) * (elf_head.e_shentsize as usize),
            );
            // println!("SectionHeaderBytes: {:?}", section_header_bytes);

            let mut elf = elf::Elf::lazy_parse(elf_head)?;
            let context = goblin::container::Ctx {
                container: elf.header.container()?,
                le: elf.header.endianness()?,
            };

            elf.section_headers = SectionHeader::parse(
                &section_header_bytes,
                0,
                elf_head.e_shnum as usize,
                context,
            )?;

            println!("SectionHeaders: {:#?}", elf.section_headers);
```

The `SectionHeader::parse()` will fail because of https://github.com/m4b/goblin/blob/5b67d29bee4fa31766cdb9ec04062503b836701e/src/elf/section_header.rs#L449-L451

It likely makes sense when parsing the whole complete file, but in my case I want to just parse this section, and the offset for that section is e.g., at `0xc8e912a0` so that's quite a few bytes to read from the file for nothing. Locally just avoiding this works very well.


Discussion / hints:

Fix patch:
diff --git a/src/elf/section_header.rs b/src/elf/section_header.rs
index 981706153..a81839eb0 100644
--- a/src/elf/section_header.rs
+++ b/src/elf/section_header.rs
@@ -442,18 +442,25 @@ if_alloc! {
             self.sh_addr as usize..(self.sh_addr as usize).saturating_add(self.sh_size as usize)
         }
         /// Parse `count` section headers from `bytes` at `offset`, using the given `ctx`
+        /// Assuming this is read from the whole file, it will check offset.
         #[cfg(feature = "endian_fd")]
-        pub fn parse(bytes: &[u8], mut offset: usize, mut count: usize, ctx: Ctx) -> error::Result<Vec<SectionHeader>> {
-            use scroll::Pread;
+        pub fn parse(bytes: &[u8], offset: usize, count: usize, ctx: Ctx) -> error::Result<Vec<SectionHeader>> {
             // Zero offset means no section headers, not even the null section header.
             if offset == 0 {
                 return Ok(Vec::new());
             }
+            Self::parse_from(bytes, offset, count, ctx)
+        }
+        /// Parse `count` section headers from `bytes` at `offset`, using the given `ctx`
+        /// without performing any offset checking to allow parsing relatively
+        #[cfg(feature = "endian_fd")]
+        pub fn parse_from(bytes: &[u8], mut offset: usize, mut count: usize, ctx: Ctx) -> error::Result<Vec<SectionHeader>> {
+            use scroll::Pread;
             let empty_sh = bytes.gread_with::<SectionHeader>(&mut offset, ctx)?;
             if count == 0 as usize {
-                // Zero count means either no section headers if offset is also zero (checked
-                // above), or the number of section headers overflows SHN_LORESERVE, in which
-                // case the count is stored in the sh_size field of the null section header.
+                // Zero count means either no section headers or the number of section headers
+                // overflows SHN_LORESERVE, in which case the count is stored in the sh_size field
+                // of the null section header.
                 count = empty_sh.sh_size as usize;
             }
 


Test patch:
diff --git a/tests/elf.rs b/tests/elf.rs
index d12c4b4c3..046d07309 100644
--- a/tests/elf.rs
+++ b/tests/elf.rs
@@ -75,6 +75,36 @@ fn parse_text_section_size_lazy(base: &[u8]) -> Result<u64, &'static str> {
     Err("Didn't find text section")
 }
 
+fn parse_from_text_section_size_lazy(base: &[u8]) -> Result<bool, &'static str> {
+    let header = Elf::parse_header(base).map_err(|_| "parse elf header error")?;
+
+    use goblin::container::{Container, Ctx};
+    use goblin::elf::SectionHeader;
+
+    let ctx = Ctx {
+        le: scroll::Endian::Little,
+        container: Container::Big,
+    };
+
+    let sh_from_orig =
+        SectionHeader::parse(base, header.e_shoff as usize, header.e_shnum as usize, ctx)
+            .map_err(|_| "parse() section headers error")?;
+
+    let sh_from_offset = SectionHeader::parse_from(
+        &base[header.e_shoff as usize..],
+        0,
+        header.e_shnum as usize,
+        ctx,
+    )
+    .map_err(|_| "parse_from() section headers error")?;
+
+    if sh_from_orig == sh_from_offset {
+        return Ok(true);
+    }
+
+    Err("Mismatching offset reading")
+}
+
 #[test]
 fn test_parse_gnu_hash_section_64bit() {
     static ALIGNED_DATA: &AlignedData<[u8]> =
@@ -148,6 +178,13 @@ fn test_parse_text_section_size_lazy() {
     assert_eq!(parse_text_section_size_lazy(&ALIGNED_DATA.0), Ok(0x126));
 }
 
+#[test]
+fn test_parse_from_text_section_size_lazy() {
+    static ALIGNED_DATA: &AlignedData<[u8]> =
+        &AlignedData(*include_bytes!("bins/elf/gnu_hash/hello.so"));
+    assert_eq!(parse_from_text_section_size_lazy(&ALIGNED_DATA.0), Ok(true));
+}
+
 #[test]
 fn test_oom() {
     use goblin::container::{Container, Ctx};