Skip to content

Commit

Permalink
feat: validate pe dll names, add size limit
Browse files Browse the repository at this point in the history
Reflect what is done in yara, where dll names are validated otherwise
ignored. A new limit was added in YARA 4.4 as well.
  • Loading branch information
vthib committed Feb 16, 2024
1 parent ee5be52 commit 51e27fd
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 1 deletion.
22 changes: 21 additions & 1 deletion boreal/src/module/pe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const MAX_PE_SECTIONS: usize = 96;
const MAX_PE_IMPORTS: usize = 16384;
const MAX_PE_EXPORTS: usize = 16384;
const MAX_EXPORT_NAME_LENGTH: usize = 512;
const MAX_IMPORT_DLL_NAME_LENGTH: usize = 256;
const MAX_RESOURCES: usize = 65536;
const MAX_NB_DATA_DIRECTORIES: usize = 32768;
const MAX_NB_VERSION_INFOS: usize = 32768;
Expand Down Expand Up @@ -1414,8 +1415,11 @@ fn add_imports<Pe: ImageNtHeaders>(
Ok(name) => name.to_vec(),
Err(_) => continue,
};
let mut data_functions = Vec::new();
if library.len() >= MAX_IMPORT_DLL_NAME_LENGTH || !dll_name_is_valid(&library) {
continue;
}

let mut data_functions = Vec::new();
let import_thunk_list = table
.thunks(import_desc.original_first_thunk.get(LE))
.or_else(|_| table.thunks(import_desc.first_thunk.get(LE)));
Expand Down Expand Up @@ -1577,6 +1581,10 @@ fn add_delay_load_imports<Pe: ImageNtHeaders>(
Ok(name) => name.to_vec(),
Err(_) => continue,
};
if !dll_name_is_valid(&library) {
continue;
}

let mut data_functions = Vec::new();
let functions = table
.thunks(import_desc.import_name_table_rva.get(LE))
Expand Down Expand Up @@ -1625,6 +1633,18 @@ fn add_delay_load_imports<Pe: ImageNtHeaders>(
]);
}

fn dll_name_is_valid(dll_name: &[u8]) -> bool {
dll_name.iter().all(|c| {
*c >= b' '
&& *c != b'\"'
&& *c != b'*'
&& *c != b'<'
&& *c != b'>'
&& *c != b'?'
&& *c != b'|'
})
}

fn add_exports(
data_dirs: &DataDirectories,
mem: &[u8],
Expand Down
8 changes: 8 additions & 0 deletions boreal/tests/assets/pe/README
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,11 @@ Then compile with:

This generates a DLL with no imports/exports, and some custom resources with names and not just IDs.
I haven't found a way to get names for languages however, not sure it is possible.

* `long_name_exporter.exe` and `long_name_importer.exe` are very simple PE files generated from C.

* `long_dll_name.exe` is `long_name_importer.exe` but with a dll name modified in a hex editor to be
over 256 characters.

* `invalid_dll_names.exe` is `pe_imports` from libyara assets, modified in a hex editor
to make a standard and delayed imported dll name invalid.
Binary file added boreal/tests/assets/pe/invalid_dll_names.exe
Binary file not shown.
Binary file added boreal/tests/assets/pe/long_dll_name.exe
Binary file not shown.
Binary file added boreal/tests/assets/pe/long_name_exporter.exe
Binary file not shown.
Binary file added boreal/tests/assets/pe/long_name_importer.exe
Binary file not shown.
44 changes: 44 additions & 0 deletions boreal/tests/it/pe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,50 @@ fn test_coverage_pe_c6f9709f() {
compare_module_values_on_file(Pe::default(), path, true, &diffs);
}

#[test]
fn test_coverage_pe_long_name_exporter() {
let diffs = [
#[cfg(not(feature = "authenticode"))]
"pe.number_of_signatures",
];
let path = "tests/assets/pe/long_name_exporter.exe";
compare_module_values_on_file(Pe::default(), path, false, &diffs);
compare_module_values_on_file(Pe::default(), path, true, &diffs);
}

#[test]
fn test_coverage_pe_long_dll_name() {
let diffs = [
#[cfg(not(feature = "authenticode"))]
"pe.number_of_signatures",
];
let path = "tests/assets/pe/long_dll_name.exe";
compare_module_values_on_file(Pe::default(), path, false, &diffs);
compare_module_values_on_file(Pe::default(), path, true, &diffs);
}

#[test]
fn test_coverage_pe_long_name_importer() {
let diffs = [
#[cfg(not(feature = "authenticode"))]
"pe.number_of_signatures",
];
let path = "tests/assets/pe/long_name_importer.exe";
compare_module_values_on_file(Pe::default(), path, false, &diffs);
compare_module_values_on_file(Pe::default(), path, true, &diffs);
}

#[test]
fn test_coverage_pe_invalid_dll_names() {
let diffs = [
#[cfg(not(feature = "authenticode"))]
"pe.number_of_signatures",
];
let path = "tests/assets/pe/invalid_dll_names.exe";
compare_module_values_on_file(Pe::default(), path, false, &diffs);
compare_module_values_on_file(Pe::default(), path, true, &diffs);
}

#[test]
#[cfg(feature = "hash")]
fn test_imphash() {
Expand Down

0 comments on commit 51e27fd

Please sign in to comment.