From 29a604e9131a6947d82cee872ed06af941fabf17 Mon Sep 17 00:00:00 2001
From: Christopher Cole <luna@spacecat.art>
Date: Sat, 29 Oct 2022 15:37:19 -0700
Subject: [PATCH] Fix File::symbol_version_table() to properly load strings
 section data

Previously, it would forget to load and then just try to get the strings section data
This would work if some other processing on the file had already loaded the string table
but not if this was the first method that wanted to inspect those strings.

This class of bugs is really an integration with the CachedReadBytes interface, so it
can only get caught if we're testing with that impl. These interface tests should be parameterized
for both interfaces.
---
 src/file.rs | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/file.rs b/src/file.rs
index 9a574d9..19cb180 100644
--- a/src/file.rs
+++ b/src/file.rs
@@ -429,7 +429,13 @@ impl<R: ReadBytesAt> File<R> {
                 let size = shdr.sh_size as usize;
                 self.reader.load_bytes_at(start..start + size)?;
 
-                Some((shdr, self.section_header_by_index(shdr.sh_link as usize)?))
+                let strs_shdr = self.section_header_by_index(shdr.sh_link as usize)?;
+                let strs_start = strs_shdr.sh_offset as usize;
+                let strs_size = strs_shdr.sh_size as usize;
+                self.reader
+                    .load_bytes_at(strs_start..strs_start + strs_size)?;
+
+                Some((shdr, strs_shdr))
             }
             // It's possible to have symbol versioning with no NEEDs if we're an object that only
             // exports defined symbols.
@@ -443,7 +449,13 @@ impl<R: ReadBytesAt> File<R> {
                 let size = shdr.sh_size as usize;
                 self.reader.load_bytes_at(start..start + size)?;
 
-                Some((shdr, self.section_header_by_index(shdr.sh_link as usize)?))
+                let strs_shdr = self.section_header_by_index(shdr.sh_link as usize)?;
+                let strs_start = strs_shdr.sh_offset as usize;
+                let strs_size = strs_shdr.sh_size as usize;
+                self.reader
+                    .load_bytes_at(strs_start..strs_start + strs_size)?;
+
+                Some((shdr, strs_shdr))
             }
             // It's possible to have symbol versioning with no DEFs if we're an object that doesn't
             // export any symbols but does use dynamic symbols from other objects.
@@ -1487,9 +1499,9 @@ mod interface_tests {
     #[test]
     fn symbol_version_table() {
         let path = std::path::PathBuf::from("tests/samples/test1");
-        let file_data = std::fs::read(path).expect("Could not read file.");
-        let slice = file_data.as_slice();
-        let mut file = File::open_stream(slice).expect("Open test1");
+        let io = std::fs::File::open(path).expect("Could not open file.");
+        let mut file =
+            File::open_stream(CachedReadBytes::new(io)).expect("Failed to open ELF stream");
         let vst = file
             .symbol_version_table()
             .expect("Failed to parse GNU symbol versions")
@@ -1502,6 +1514,17 @@ mod interface_tests {
         assert_eq!(req1.file, "libc.so.6");
         assert_eq!(req1.name, "GLIBC_2.2.5");
         assert_eq!(req1.hash, 0x9691A75);
+
+        let req2 = vst
+            .get_requirement(2)
+            .expect("Failed to parse NEED")
+            .expect("Failed to find NEED");
+        assert_eq!(req2.file, "libc.so.6");
+        assert_eq!(req2.name, "GLIBC_2.2.5");
+        assert_eq!(req2.hash, 0x9691A75);
+
+        let req3 = vst.get_requirement(3).expect("Failed to parse NEED");
+        assert!(req3.is_none());
     }
 }