diff --git a/boreal/src/module/pe.rs b/boreal/src/module/pe.rs index 4af10133..33aa34a6 100644 --- a/boreal/src/module/pe.rs +++ b/boreal/src/module/pe.rs @@ -1836,6 +1836,8 @@ fn add_resources( data: &mut Data, out: &mut HashMap<&'static str, Value>, ) { + let mut infos = Vec::new(); + let dir = data_dirs.resource_directory(mem, sections).ok().flatten(); let root = dir.as_ref().and_then(|dir| dir.root().ok()); if let (Some(dir), Some(root)) = (dir, root) { @@ -1876,7 +1878,7 @@ fn add_resources( let offset = va_to_file_offset(mem, sections, rva); if ty == u32::from(pe::RT_VERSION) { if let Some(offset) = offset { - add_version_infos(mem, offset, out); + version_info::read_version_info(mem, offset as usize, &mut infos); } } @@ -1937,12 +1939,6 @@ fn add_resources( } else { let _r = out.insert("number_of_resources", Value::Integer(0)); } -} - -pub fn add_version_infos(mem: &[u8], offset: u32, out: &mut HashMap<&'static str, Value>) { - let Some(infos) = version_info::read_version_info(mem, offset as usize) else { - return; - }; out.extend([ ("number_of_version_infos", infos.len().into()), diff --git a/boreal/src/module/pe/version_info.rs b/boreal/src/module/pe/version_info.rs index 7e74333a..76a06124 100644 --- a/boreal/src/module/pe/version_info.rs +++ b/boreal/src/module/pe/version_info.rs @@ -7,43 +7,48 @@ pub struct VersionInfo { pub value: Vec, } -pub fn read_version_info(mem: &[u8], offset: usize) -> Option> { +pub fn read_version_info(mem: &[u8], mut offset: usize, infos: &mut Vec) { const VS_VERSION_INFO_KEY: &[u8] = b"V\0S\0_\0V\0E\0R\0S\0I\0O\0N\0_\0I\0N\0F\0O\0\0\0"; let mut bytes = Bytes(mem); - bytes.skip(offset).ok()?; - let header = read_header(&mut bytes)?; + if bytes.skip(offset).is_err() { + return; + } + let Some(header) = read_header(&mut bytes) else { + return; + }; + let version_info_length = usize::from(header.length); let end = offset + version_info_length; - let key = bytes.read_bytes(VS_VERSION_INFO_KEY.len()).ok()?; + let Ok(key) = bytes.read_bytes(VS_VERSION_INFO_KEY.len()) else { + return; + }; if key.0 != VS_VERSION_INFO_KEY { - return None; + return; } // VS_FIXEDFILEINFO is 32-bits aligned after the key. - let offset = align32(offset + HEADER_SIZE + VS_VERSION_INFO_KEY.len()); + // let offset = align32(offset + HEADER_SIZE + VS_VERSION_INFO_KEY.len()); // Then add the length of the VS_FIXEDFILEINFO, and realign - let mut offset = align32(offset + usize::from(header.value_length)); + // let mut offset = align32(offset + usize::from(header.value_length)); + offset += align32(HEADER_SIZE + 86); // Skip VarFileInfo, if any while let Some(length) = read_var_file_info(mem, offset) { - offset = align32(offset + length); + offset += align32(length); } // Then read StringFileInfo - let mut infos = Vec::new(); while offset < end { if infos.len() >= MAX_NB_VERSION_INFOS { break; } - match read_string_file_info(mem, offset, &mut infos) { - Some(length) => offset = align32(offset + length), + match read_string_file_info(mem, offset, infos) { + Some(length) => offset += length, None => break, } } - - Some(infos) } // Read a VarFileInfo. If present, return the length of the entry @@ -62,7 +67,11 @@ fn read_var_file_info(mem: &[u8], offset: usize) -> Option { } // Read a StringFileInfo. If present, return the length of the entry -fn read_string_file_info(mem: &[u8], offset: usize, out: &mut Vec) -> Option { +fn read_string_file_info( + mem: &[u8], + mut offset: usize, + out: &mut Vec, +) -> Option { const STRING_FILE_INFO_KEY: &[u8] = b"S\0t\0r\0i\0n\0g\0F\0i\0l\0e\0I\0n\0f\0o\0\0\0"; let mut bytes = Bytes(mem); @@ -73,11 +82,11 @@ fn read_string_file_info(mem: &[u8], offset: usize, out: &mut Vec) return None; } - let length = usize::from(header.length); + let length = align32(usize::from(header.length)); let end = offset + length; // StringTable is then aligned - let mut offset = align32(offset + HEADER_SIZE + STRING_FILE_INFO_KEY.len()); + offset += align32(HEADER_SIZE + STRING_FILE_INFO_KEY.len()); while offset < end { if out.len() >= MAX_NB_VERSION_INFOS { @@ -85,7 +94,7 @@ fn read_string_file_info(mem: &[u8], offset: usize, out: &mut Vec) } match read_string_table(mem, offset, out) { - Some(length) => offset = align32(offset + length), + Some(length) => offset += length, None => break, } } @@ -94,16 +103,17 @@ fn read_string_file_info(mem: &[u8], offset: usize, out: &mut Vec) } // Read a StringTable. If present, return the length of the entry -fn read_string_table(mem: &[u8], offset: usize, out: &mut Vec) -> Option { +fn read_string_table(mem: &[u8], mut offset: usize, out: &mut Vec) -> Option { let mut bytes = Bytes(mem); bytes.skip(offset).ok()?; let header = read_header(&mut bytes)?; - let length = usize::from(header.length); + let length = align32(usize::from(header.length)); let end = offset + length; + let key_len = find_wide_nul(bytes.0); // move to children: header, 8 byte wide string and padding - let mut offset = align32(offset + HEADER_SIZE + 2 * 9); + offset += align32(HEADER_SIZE + key_len + 2); while offset < end { if out.len() >= MAX_NB_VERSION_INFOS { @@ -111,7 +121,7 @@ fn read_string_table(mem: &[u8], offset: usize, out: &mut Vec) -> O } match read_string(mem, offset, out) { - Some(length) => offset = align32(offset + length), + Some(length) => offset += length, None => break, } } @@ -125,26 +135,39 @@ fn read_string(mem: &[u8], offset: usize, out: &mut Vec) -> Option< bytes.skip(offset).ok()?; let header = read_header(&mut bytes)?; - let length = usize::from(header.length); + let length = align32(usize::from(header.length)); let end = offset + length; - // We have the size for the value, hence easily getting the value. - let value_length = usize::from(header.value_length); - // This length is in characters, multiply by 2 to get byte length. - let value_length = value_length * 2; - if value_length + 6 > length || end < value_length { + let mem = mem.get(offset..end)?; + + // The shape of the struct is normally this: + // + // - length of struct + // - value length + // - type + // - wide key + // - padding to 32bits + // - value + // + // The `value_length` is supposed to indicate the length of the value: + // + // - number of characters for the value if type == 1 + // - number of bytes for the value if type == 0 + // + // This "type == 0" is weird, some files uses it but still put a wide string + // in the value. + // + // To be as permissive as possible, we ignore value length and type: we simply + // get the key, remove the padding, and get the value, as two wide strings. + let key_start = HEADER_SIZE; + if key_start > mem.len() { return None; } - - // The payload is the key followed by the value. We have the length of the value, but - // it is relative to the end of the key (the overall length can be greater than key + value - // and padded with nul bytes). - // So we need to find the end of the key first. - let key_start = offset + HEADER_SIZE; - let key_end = key_start + find_wide_nul(&mem[key_start..(end - value_length)]); - let value_start = key_end + 2; - if value_start + value_length > end { + let key_end = key_start + find_wide_nul(mem.get(key_start..)?); + let value_start = align32(key_end + 2); + if value_start > end { return None; } + let value_end = value_start + find_wide_nul(mem.get(value_start..)?); // Those are windows wide strings. We should technically: // - read it into a u16 slice @@ -153,7 +176,7 @@ fn read_string(mem: &[u8], offset: usize, out: &mut Vec) -> Option< // But yara simply strips the second byte of every pair (expecting it to always be 0). We could // differ here, but for the moment keep this broken behavior let key = unwide(&mem[key_start..key_end]); - let value = unwide(&mem[value_start..(value_start + value_length)]); + let value = unwide(&mem[value_start..value_end]); out.push(VersionInfo { key, value }); @@ -161,14 +184,14 @@ fn read_string(mem: &[u8], offset: usize, out: &mut Vec) -> Option< } fn find_wide_nul(mem: &[u8]) -> usize { - let mut i = mem.len(); - while i >= 2 { - i -= 2; + let mut i = 0; + while i + 1 < mem.len() { if mem[i] == b'\0' && mem[i + 1] == b'\0' { return i; } + i += 2; } - 0 + mem.len() } fn unwide(mem: &[u8]) -> Vec {