Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: add checks on file read for linux process scan
Browse files Browse the repository at this point in the history
vthib committed Dec 17, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 6c5ab08 commit 91a4906
Showing 3 changed files with 158 additions and 24 deletions.
89 changes: 76 additions & 13 deletions boreal-test-helpers/src/main.rs
Original file line number Diff line number Diff line change
@@ -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;
}
}
55 changes: 44 additions & 11 deletions boreal/src/scanner/process/sys/linux.rs
Original file line number Diff line number Diff line change
@@ -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>(
38 changes: 38 additions & 0 deletions boreal/tests/it/process.rs
Original file line number Diff line number Diff line change
@@ -317,6 +317,44 @@ rule a {
);
}

#[test]
#[cfg(any(target_os = "linux", windows))]
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>,

0 comments on commit 91a4906

Please sign in to comment.