feat: add checks on file read for linux process scan

boreal-test-helpers/src/main.rs

@@ -16,21 +16,11 @@ fn main() {
         "max_fetched_region_size" => max_fetched_region_size(),
         "memory_chunk_size" => memory_chunk_size(),
         "file_copy_on_write" => file_copy_on_write(),
+        "rework_file" => rework_file(),
         _ => panic!("unknown arg {}", arg),
     }
 }
 
-struct Region {
-    _file: tempfile::NamedTempFile,
-    map: memmap2::MmapMut,
-}
-
-impl Region {
-    fn addr(&self) -> usize {
-        self.map.as_ptr() as usize
-    }
-}
-
 fn stack() {
     // This is "test0123456789helper" when xor'ed
     let payload = xor_bytes(b"{j|{?>=<;:9876gjc\x7fj}", 15);
@@ -137,6 +127,68 @@ fn file_copy_on_write() {
     std::thread::sleep(std::time::Duration::from_secs(500));
 }
 
+fn rework_file() {
+    // Pattern is "z8Nwed8LTu"
+    let bad = b"u7Axjk7C[z";
+    // Pattern is "i7B7hm8PoV"
+    let good = b"f8M8gb7_`Y";
+
+    // Region 1: replace the backing file after mapping
+
+    // Create a region backed by a file, and write the good pattern
+    let mut file = tempfile::NamedTempFile::new().unwrap();
+    let mut contents = vec![0; 2 * PAGE_SIZE];
+    let offset = PAGE_SIZE + 100;
+    xor_bytes_into(good, 15, &mut contents[offset..(offset + good.len())]);
+    file.write_all(&contents).unwrap();
+    let path = file.path().to_path_buf();
+    let map1 = unsafe { memmap2::MmapMut::map_mut(file.as_file()).unwrap() };
+    erase(contents);
+
+    // Drop the file, and write an new file in its place, with the bad pattern.
+    std::mem::drop(file);
+
+    // Since the file has been dropped, the maps file will contain `<path> (deleted)`.
+    // We can actually create a file with this suffix to try and trick the scanner.
+    let mut path = path.display().to_string();
+    path.push_str(" (deleted)");
+    let mut new_file = std::fs::File::create(&path).unwrap();
+    let mut new_contents = vec![0; PAGE_SIZE];
+    let new_offset = PAGE_SIZE - 100;
+    xor_bytes_into(
+        bad,
+        15,
+        &mut new_contents[new_offset..(new_offset + bad.len())],
+    );
+    new_file.write_all(&new_contents).unwrap();
+    erase(new_contents);
+
+    // Region 2: shorten the file prior to the offset.
+    let mut contents = vec![0; 3 * PAGE_SIZE];
+    let offset = 3 * PAGE_SIZE - 300;
+    xor_bytes_into(bad, 15, &mut contents[offset..(offset + bad.len())]);
+    let region2 = Region::copy_on_write(contents, 2 * PAGE_SIZE as u64);
+    region2.file.as_file().set_len((PAGE_SIZE) as u64).unwrap();
+
+    // Region 3: shorten the backing file after mapping
+    let mut region3 = Region::zeroed(3 * PAGE_SIZE);
+    region3.write_at(PAGE_SIZE - 500, good);
+    region3.write_at(3 * PAGE_SIZE - 500, bad);
+    region3.file.as_file().set_len((PAGE_SIZE) as u64).unwrap();
+
+    println!("{:x}", map1.as_ptr() as usize);
+    println!("{:x}", region2.addr());
+    println!("{:x}", region3.addr());
+
+    println!("ready");
+    std::thread::sleep(std::time::Duration::from_secs(500));
+}
+
+struct Region {
+    file: tempfile::NamedTempFile,
+    map: memmap2::MmapMut,
+}
+
 impl Region {
     fn new(contents: &[u8]) -> Self {
         let mut this = Self::zeroed(contents.len());
@@ -150,7 +202,7 @@ impl Region {
         file.write_all(&contents).unwrap();
         let map = unsafe { memmap2::MmapMut::map_mut(file.as_file()).unwrap() };
 
-        Self { _file: file, map }
+        Self { file, map }
     }
 
     fn copy_on_write(mut contents: Vec<u8>, offset: u64) -> Self {
@@ -170,12 +222,16 @@ impl Region {
                 .unwrap()
         };
 
-        Self { _file: file, map }
+        Self { file, map }
     }
 
     fn write_at(&mut self, offset: usize, payload: &[u8]) {
         xor_bytes_into(payload, 15, &mut self.map[offset..(offset + payload.len())]);
     }
+
+    fn addr(&self) -> usize {
+        self.map.as_ptr() as usize
+    }
 }
 
 fn xor_bytes(v: &[u8], xor_byte: u8) -> Vec<u8> {
@@ -187,3 +243,10 @@ fn xor_bytes_into(v: &[u8], xor_byte: u8, dest: &mut [u8]) {
         *d = *v ^ xor_byte;
     }
 }
+
+fn erase(mut contents: Vec<u8>) {
+    // Erase contents to not let it live in our RAM.
+    for b in &mut contents {
+        *b = 0;
+    }
+}

boreal/src/scanner/process/sys/linux.rs

@@ -1,5 +1,6 @@
 use std::fs::File;
 use std::io::{BufRead, BufReader, Read, Seek, SeekFrom};
+use std::os::unix::fs::MetadataExt;
 use std::path::{Path, PathBuf};
 
 use crate::memory::{FragmentedMemory, MemoryParams, Region, RegionDescription};
@@ -192,6 +193,45 @@ struct MapRegion {
     path: Option<PathBuf>,
 }
 
+impl MapRegion {
+    fn open_file(&self) -> Option<(File, usize)> {
+        let check_metadata = |metadata: std::fs::Metadata| -> bool {
+            let dev = metadata.dev();
+            // Safety: this is always safe to call.
+            let (dmaj, dmin) = unsafe { (libc::major(dev), libc::minor(dev)) };
+
+            metadata.ino() == self.inode
+                && dmaj == self.dev_major
+                && dmin == self.dev_minor
+                && metadata.mode() & libc::S_IFMT == libc::S_IFREG
+                && metadata.len() > self.offset
+        };
+
+        // Do not try to map the file if it does not belong to any device
+        if self.dev_major == 0 && self.dev_minor == 0 {
+            return None;
+        }
+
+        let path = self.path.as_ref()?;
+        // First, do a sanity check on the metadata of the path, this avoids opening
+        // the file if it does not pass those checks.
+        if !check_metadata(std::fs::metadata(path).ok()?) {
+            return None;
+        }
+
+        let file = std::fs::File::open(path).ok()?;
+
+        // Then, redo the checks but on the metadata of the opened files, to prevent
+        // any TOCTOU issues.
+        if !check_metadata(file.metadata().ok()?) {
+            return None;
+        }
+
+        let file_size = file.metadata().ok()?.len().try_into().ok()?;
+        Some((file, file_size))
+    }
+}
+
 /// Description of a region currently being listed or fetched
 #[derive(Debug)]
 struct CurrentRegion {
@@ -249,17 +289,10 @@ impl CurrentRegion {
         if self.file.is_some() {
             return;
         }
-        let Some(path) = &self.desc.path else {
-            return;
-        };
-        let Ok(file) = std::fs::File::open(path) else {
-            return;
-        };
-        let Some(file_size) = file.metadata().ok().and_then(|v| v.len().try_into().ok()) else {
-            return;
-        };
-        self.file_size = file_size;
-        self.file = Some(file);
+        if let Some((file, file_size)) = self.desc.open_file() {
+            self.file = Some(file);
+            self.file_size = file_size;
+        }
     }
 
     fn fetch<'a>(

boreal/tests/it/process.rs

@@ -317,6 +317,44 @@ rule a {
     );
 }
 
+#[test]
+#[cfg(target_os = "linux")]
+fn test_process_rework_file() {
+    let mut checker = Checker::new(
+        r#"
+rule good {
+    strings:
+        $good = "i7B7hm8PoV"
+    condition:
+        $good
+}
+
+rule bad {
+    strings:
+        $bad = "z8Nwed8LTu"
+    condition:
+        $bad
+}"#,
+    );
+
+    let helper = BinHelper::run("rework_file");
+    assert_eq!(helper.output.len(), 3);
+    let region1 = usize::from_str_radix(&helper.output[0], 16).unwrap();
+    let _region2 = usize::from_str_radix(&helper.output[1], 16).unwrap();
+    let region3 = usize::from_str_radix(&helper.output[2], 16).unwrap();
+
+    let mut expected = vec![
+        (b"i7B7hm8PoV".as_slice(), region1 + PAGE_SIZE + 100, 10),
+        (b"i7B7hm8PoV".as_slice(), region3 + PAGE_SIZE - 500, 10),
+    ];
+    expected.sort_by_key(|v| v.1);
+
+    checker.check_process_full_matches(
+        helper.pid(),
+        vec![("default:good".to_owned(), vec![("good", expected)])],
+    );
+}
+
 struct BinHelper {
     proc: std::process::Child,
     output: Vec<String>,