diff --git a/boreal/src/module/pe.rs b/boreal/src/module/pe.rs index 2e38a36f..b1de1f46 100644 --- a/boreal/src/module/pe.rs +++ b/boreal/src/module/pe.rs @@ -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; @@ -1414,8 +1415,11 @@ fn add_imports( 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))); @@ -1577,6 +1581,10 @@ fn add_delay_load_imports( 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)) @@ -1625,6 +1633,18 @@ fn add_delay_load_imports( ]); } +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], diff --git a/boreal/tests/assets/pe/README b/boreal/tests/assets/pe/README index 21b276ea..28d0a6fc 100644 --- a/boreal/tests/assets/pe/README +++ b/boreal/tests/assets/pe/README @@ -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. diff --git a/boreal/tests/assets/pe/invalid_dll_names.exe b/boreal/tests/assets/pe/invalid_dll_names.exe new file mode 100644 index 00000000..4dfbed91 Binary files /dev/null and b/boreal/tests/assets/pe/invalid_dll_names.exe differ diff --git a/boreal/tests/assets/pe/long_dll_name.exe b/boreal/tests/assets/pe/long_dll_name.exe new file mode 100755 index 00000000..30dc11c0 Binary files /dev/null and b/boreal/tests/assets/pe/long_dll_name.exe differ diff --git a/boreal/tests/assets/pe/long_name_exporter.exe b/boreal/tests/assets/pe/long_name_exporter.exe new file mode 100755 index 00000000..b68ab040 Binary files /dev/null and b/boreal/tests/assets/pe/long_name_exporter.exe differ diff --git a/boreal/tests/assets/pe/long_name_importer.exe b/boreal/tests/assets/pe/long_name_importer.exe new file mode 100755 index 00000000..8bb79323 Binary files /dev/null and b/boreal/tests/assets/pe/long_name_importer.exe differ diff --git a/boreal/tests/it/pe.rs b/boreal/tests/it/pe.rs index 32510ebb..8bcc3990 100644 --- a/boreal/tests/it/pe.rs +++ b/boreal/tests/it/pe.rs @@ -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() {