From db2729737f947137583288f90d83cf578d33243a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B6=BE=E7=80=AC=E6=A1=83=E6=A1=83?= Date: Wed, 4 Nov 2026 12:31:21 +0800 Subject: [PATCH] fix(windows): validate DLL layout candidates before selection --- senbei-engine/src/windows/error.rs | 3 + senbei-engine/src/windows/integrity.rs | 225 ++++++++++---- senbei-engine/src/windows/mod.rs | 387 +++++++++++++++++++++++-- 3 files changed, 532 insertions(+), 83 deletions(-) diff --git a/senbei-engine/src/windows/error.rs b/senbei-engine/src/windows/error.rs index 09bb0d5..3ce4a33 100644 --- a/senbei-engine/src/windows/error.rs +++ b/senbei-engine/src/windows/error.rs @@ -224,6 +224,9 @@ pub enum UnpackError { exe: Box, }, + #[error("structurally invalid layout candidate: {}", issues.join("; "))] + InvalidLayoutCandidate { issues: Vec }, + #[error( "PE32 second-stage range is invalid (offset {offset}, size {size}, image length {image_len})" )] diff --git a/senbei-engine/src/windows/integrity.rs b/senbei-engine/src/windows/integrity.rs index 6ae8dcd..7b0257c 100644 --- a/senbei-engine/src/windows/integrity.rs +++ b/senbei-engine/src/windows/integrity.rs @@ -13,11 +13,44 @@ //! //! All reads are bounds-checked; the check never panics on any input. +/// How serious one [`IntegrityReport`] defect is for choosing a DLL layout. +/// +/// This does not change what [`check`] puts in [`IntegrityReport::issues`]. +/// Callers that surface suspect output still read that list. Layout selection +/// uses [`IntegrityFinding::severity`] so a warning (the image may still be the +/// right layout) is not treated as "try the other pipeline". +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IntegritySeverity { + /// The loader or runtime rejects the image before executing it, or a + /// directory the header claims to own does not resolve inside the file. + /// A DLL layout that reports one of these is the wrong layout. + Fatal, + /// The image is a PE, but a recoverable region looks unrestored. Real + /// outputs can carry these and still be the layout that must be kept + /// (a native DLL with no base relocations, an entry stub left as zeros + /// or `int3`). + Warning, +} + +/// One defect from [`check`], with the severity layout selection needs. +/// +/// `message` is the same string stored in [`IntegrityReport::issues`]. Severity +/// is decided while the check still knows which rule fired, not by reading the +/// message back. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IntegrityFinding { + pub severity: IntegritySeverity, + pub message: String, +} + /// Result of a static integrity check over an unpacked image. #[derive(Debug, Clone, Default)] pub struct IntegrityReport { /// Each entry describes one detected defect. Empty means no defect found. + /// Order and wording match [`IntegrityReport::findings`]. pub issues: Vec, + /// The same defects as `issues`, each tagged fatal or warning. + pub findings: Vec, } impl IntegrityReport { @@ -25,6 +58,23 @@ impl IntegrityReport { pub fn ok(&self) -> bool { self.issues.is_empty() } + + /// True when every defect is a [`IntegritySeverity::Warning`]. + /// + /// An empty report is not fatal. Layout selection uses this to keep a DLL + /// image whose only defects are warnings, and to reject one the loader + /// cannot map. + pub fn has_fatal(&self) -> bool { + self.findings + .iter() + .any(|finding| finding.severity == IntegritySeverity::Fatal) + } + + fn push(&mut self, severity: IntegritySeverity, message: impl Into) { + let message = message.into(); + self.issues.push(message.clone()); + self.findings.push(IntegrityFinding { severity, message }); + } } // `checked_add`, not `+`: `usize` is 32-bit on wasm32, so a header-derived @@ -92,9 +142,12 @@ fn check_common_entry_branches( .ok() .is_some_and(|rva| is_executable_rva(secs, rva)); if !valid { - report.issues.push(format!( - "entry point {name} target 0x{target:X} is outside executable sections (DD8 selection is likely wrong)" - )); + report.push( + IntegritySeverity::Fatal, + format!( + "entry point {name} target 0x{target:X} is outside executable sections (DD8 selection is likely wrong)" + ), + ); } } } @@ -107,26 +160,31 @@ pub fn check(out: &[u8]) -> IntegrityReport { // --- DOS + PE headers --------------------------------------------------- if rd_u16(out, 0) != Some(0x5A4D) { - r.issues.push("missing 'MZ' DOS signature".into()); + r.push(IntegritySeverity::Fatal, "missing 'MZ' DOS signature"); return r; // nothing else is meaningful } let pe_off = match rd_u32(out, 0x3C) { Some(v) => v, None => { - r.issues.push("truncated DOS header (no e_lfanew)".into()); + r.push( + IntegritySeverity::Fatal, + "truncated DOS header (no e_lfanew)", + ); return r; } }; if rd_u32(out, pe_off) != Some(0x0000_4550) { - r.issues - .push(format!("missing 'PE\\0\\0' signature at 0x{pe_off:X}")); + r.push( + IntegritySeverity::Fatal, + format!("missing 'PE\\0\\0' signature at 0x{pe_off:X}"), + ); return r; } let num_sections = match rd_u16(out, pe_off.wrapping_add(6)) { Some(v) => v as u32, None => { - r.issues.push("truncated COFF header".into()); + r.push(IntegritySeverity::Fatal, "truncated COFF header"); return r; } }; @@ -134,7 +192,7 @@ pub fn check(out: &[u8]) -> IntegrityReport { let magic = match rd_u16(out, opt) { Some(v) => v, None => { - r.issues.push("truncated optional header".into()); + r.push(IntegritySeverity::Fatal, "truncated optional header"); return r; } }; @@ -142,36 +200,44 @@ pub fn check(out: &[u8]) -> IntegrityReport { 0x20B => true, 0x10B => false, other => { - r.issues - .push(format!("bad optional-header magic 0x{other:X}")); + r.push( + IntegritySeverity::Fatal, + format!("bad optional-header magic 0x{other:X}"), + ); return r; } }; if num_sections == 0 || num_sections > 96 { - r.issues - .push(format!("implausible section count {num_sections}")); + r.push( + IntegritySeverity::Fatal, + format!("implausible section count {num_sections}"), + ); } let size_of_image = rd_u32(out, pe_off.wrapping_add(80)).unwrap_or(0); if size_of_image == 0 { - r.issues.push("SizeOfImage is zero".into()); + r.push(IntegritySeverity::Fatal, "SizeOfImage is zero"); } // --- Section table ------------------------------------------------------ let headers = match senbei_pe::parse(out) { Ok(headers) => headers, Err(_) => { - r.issues - .push("section table extends past end of file".into()); + r.push( + IntegritySeverity::Fatal, + "section table extends past end of file", + ); return r; } }; let parsed_sections = match senbei_pe::sections(out, headers) { Ok(sections) => sections, Err(_) => { - r.issues - .push("section table extends past end of file".into()); + r.push( + IntegritySeverity::Fatal, + "section table extends past end of file", + ); return r; } }; @@ -182,10 +248,13 @@ pub fn check(out: &[u8]) -> IntegrityReport { if section.raw_size != 0 { let end = section.raw_offset.wrapping_add(section.raw_size) as usize; if end > file_len { - r.issues.push(format!( - "section #{i} raw data [0x{:X}..0x{end:X}] exceeds file size 0x{file_len:X}", - section.raw_offset - )); + r.push( + IntegritySeverity::Fatal, + format!( + "section #{i} raw data [0x{:X}..0x{end:X}] exceeds file size 0x{file_len:X}", + section.raw_offset + ), + ); } } section @@ -222,9 +291,13 @@ pub fn check(out: &[u8]) -> IntegrityReport { if is_dll && !is_managed { let reloc_rva = rd_u32(out, dd_base.wrapping_add(5 * 8)).unwrap_or(0); if reloc_rva == 0 { - r.issues.push( - "native DLL has no base relocation table (DD[5] is zero) — will crash when loaded at a non-preferred base" - .into(), + // Warning, not fatal: a native DLL can ship without base relocations + // (`IMAGE_FILE_RELOCS_STRIPPED`, or a module that is only ever loaded + // at its preferred base). Rejecting the layout here would fall back + // to the other pipeline and change bytes that previously unpacked. + r.push( + IntegritySeverity::Warning, + "native DLL has no base relocation table (DD[5] is zero) — will crash when loaded at a non-preferred base", ); } } @@ -237,28 +310,38 @@ pub fn check(out: &[u8]) -> IntegrityReport { if ep == 0 { // A DLL may legitimately have no entry point; an EXE never does. if !is_dll { - r.issues.push("entry point RVA is zero".into()); + r.push(IntegritySeverity::Fatal, "entry point RVA is zero"); } } else if !is_managed { match rva_to_off(out, headers, ep, 16) { None => { - r.issues.push(format!( - "entry point RVA 0x{ep:X} does not map into any section" - )); + r.push( + IntegritySeverity::Fatal, + format!("entry point RVA 0x{ep:X} does not map into any section"), + ); } Some(off) => { let stub = &out[off as usize..off as usize + 16]; if stub.iter().all(|&b| b == 0) { - r.issues.push(format!( - "entry point at RVA 0x{ep:X} is all zeros (stub not recovered)" - )); + // Warning: the entry maps and is executable, but its bytes + // were not recovered. That is the suspect signal for an + // already-chosen layout, not proof another layout is the + // right one. + r.push( + IntegritySeverity::Warning, + format!("entry point at RVA 0x{ep:X} is all zeros (stub not recovered)"), + ); } else if stub.iter().all(|&b| b == 0xCC) { // 16 bytes of int3 padding where the entry stub should be: // the stub region was never recovered, the loader walks - // straight into a debug-break wall. - r.issues.push(format!( - "entry point at RVA 0x{ep:X} is all int3 padding (stub not recovered)" - )); + // straight into a debug-break wall. Same severity as a + // zeroed stub: the image is mapped, the bytes look unrestored. + r.push( + IntegritySeverity::Warning, + format!( + "entry point at RVA 0x{ep:X} is all int3 padding (stub not recovered)" + ), + ); } // The entry must live in an executable section. let exec = secs.iter().any(|s| { @@ -268,9 +351,10 @@ pub fn check(out: &[u8]) -> IntegrityReport { && (s.characteristics & 0x2000_0000) != 0 }); if !exec { - r.issues.push(format!( - "entry point RVA 0x{ep:X} is not in an executable section" - )); + r.push( + IntegritySeverity::Fatal, + format!("entry point RVA 0x{ep:X} is not in an executable section"), + ); } if let Some(entry_stub) = out.get(off as usize..off as usize + 18) { check_common_entry_branches(entry_stub, ep, &secs, &mut r); @@ -293,9 +377,10 @@ pub fn check(out: &[u8]) -> IntegrityReport { let imp_rva = rd_u32(out, dd_base.wrapping_add(8)).unwrap_or(0); if imp_rva != 0 { match rva_to_off(out, headers, imp_rva, 20) { - None => r.issues.push(format!( - "import directory RVA 0x{imp_rva:X} does not map into any section" - )), + None => r.push( + IntegritySeverity::Fatal, + format!("import directory RVA 0x{imp_rva:X} does not map into any section"), + ), Some(desc_off) => { // 256 descriptors is far beyond any real import table; the // cap keeps a corrupt, never-null table from walking on. @@ -308,14 +393,24 @@ pub fn check(out: &[u8]) -> IntegrityReport { break; } match rva_to_off(out, headers, name_rva, 1) { - None => r.issues.push(format!( - "import descriptor {i} DLL name RVA 0x{name_rva:X} does not map into any section" - )), + None => r.push( + IntegritySeverity::Fatal, + format!( + "import descriptor {i} DLL name RVA 0x{name_rva:X} does not map into any section" + ), + ), Some(noff) => { if !looks_like_dll_name(out, noff) { - r.issues.push(format!( - "import descriptor {i} DLL name at RVA 0x{name_rva:X} is not readable ASCII (imports left encrypted?)" - )); + // Warning: the name is inside the image, so + // the directory is structurally present. + // Unreadable bytes are the "left encrypted" + // suspect signal, not an unmapped PE. + r.push( + IntegritySeverity::Warning, + format!( + "import descriptor {i} DLL name at RVA 0x{name_rva:X} is not readable ASCII (imports left encrypted?)" + ), + ); } } } @@ -336,29 +431,39 @@ pub fn check(out: &[u8]) -> IntegrityReport { // begins with the "BSJB" signature. if is_managed { match rva_to_off(out, headers, clr_rva, 0x48) { - None => r.issues.push(format!( - "CLR (COR20) directory RVA 0x{clr_rva:X} does not map into any section" - )), + None => r.push( + IntegritySeverity::Fatal, + format!("CLR (COR20) directory RVA 0x{clr_rva:X} does not map into any section"), + ), Some(coff) => { let cb = rd_u32(out, coff).unwrap_or(0); if cb != 0x48 { - r.issues.push(format!( - "COR20 header at RVA 0x{clr_rva:X} has cb 0x{cb:X} (expected 0x48) — CLR header corrupt" - )); + r.push( + IntegritySeverity::Fatal, + format!( + "COR20 header at RVA 0x{clr_rva:X} has cb 0x{cb:X} (expected 0x48) — CLR header corrupt" + ), + ); } else { // MetaData RVA/size live at COR20 + 0x08 / + 0x0C. let md_rva = rd_u32(out, coff.wrapping_add(8)).unwrap_or(0); if md_rva != 0 { match rva_to_off(out, headers, md_rva, 4) { - None => r.issues.push(format!( - "CLR MetaData RVA 0x{md_rva:X} does not map into any section" - )), + None => r.push( + IntegritySeverity::Fatal, + format!( + "CLR MetaData RVA 0x{md_rva:X} does not map into any section" + ), + ), Some(moff) => { let sig = out.get(moff as usize..moff as usize + 4); if sig != Some(b"BSJB") { - r.issues.push(format!( - "CLR MetaData at RVA 0x{md_rva:X} lacks 'BSJB' signature (metadata corrupt — managed image will not load)" - )); + r.push( + IntegritySeverity::Fatal, + format!( + "CLR MetaData at RVA 0x{md_rva:X} lacks 'BSJB' signature (metadata corrupt — managed image will not load)" + ), + ); } } } diff --git a/senbei-engine/src/windows/mod.rs b/senbei-engine/src/windows/mod.rs index 29ca629..33789cf 100644 --- a/senbei-engine/src/windows/mod.rs +++ b/senbei-engine/src/windows/mod.rs @@ -15,7 +15,9 @@ pub use crate::thread_cap; pub use dll::{unpack_dll, unpack_dll_v}; pub use error::*; pub use exe::{unpack as unpack_exe, unpack_v as unpack_exe_v}; -pub use integrity::{IntegrityReport, check as check_integrity}; +pub use integrity::{ + IntegrityFinding, IntegrityReport, IntegritySeverity, check as check_integrity, +}; /// Maximum plausible PE `SizeOfImage` we are willing to allocate a zero buffer /// for. Guards against a corrupt/crafted header requesting a multi-gigabyte @@ -303,32 +305,71 @@ pub fn unpack_auto_v(input: &[u8], verbose: bool) -> Result<(Kind, Vec), Unp let detected = detect(input).ok_or(UnpackError::NotCrackproof)?; let out = match detected.kind { Kind::NativeExe | Kind::ManagedExe => unpack_exe_v(input, verbose)?, - Kind::NativeDll | Kind::ManagedDll => { - // Two Crackproof DLL layouts exist. The older one (the byte-identical - // DLL goldens) follows the pipeline in `dll.rs`. Newer builds protect - // DLLs with the EXE-style shell layout instead — `dll::unpack_dll` - // cannot parse them and errors. Try the DLL pipeline first; on - // failure, fall back to the EXE pipeline, which handles the new - // layout (including managed-DLL CLR metadata restore). The DLL-first - // order keeps the old-layout goldens byte-identical (the EXE - // pipeline "succeeds" on them but with different bytes). - match dll::unpack_dll_v(input, verbose) { - Ok(out) => out, - Err(dll_err) => match exe::unpack_v(input, verbose) { - Ok(out) => out, - Err(exe_err) => { - return Err(UnpackError::PipelineFallbackFailed { - dll: Box::new(dll_err), - exe: Box::new(exe_err), - }); - } - }, - } - } + Kind::NativeDll | Kind::ManagedDll => select_dll_layout( + || dll::unpack_dll_v(input, verbose), + || exe::unpack_v(input, verbose), + )?, }; Ok((detected.kind, out)) } +/// Pick between the older DLL layout and the EXE-style shell layout. +/// +/// `Ok` from a pipeline is not proof it chose the right layout: a wrong layout +/// can emit a PE-shaped image the loader would reject. Each successful image is +/// judged with [`dll_layout_invalid`]. The older DLL layout is kept whenever it +/// is structurally acceptable, including when its only defects are ordinary +/// warnings (a missing base-relocation directory is one of those). The EXE-style +/// layout runs only when the DLL layout errors or is structurally invalid. +/// +/// The two closures exist so tests can inject synthetic images without running +/// either real pipeline. Production passes `dll::unpack_dll_v` and `exe::unpack_v`. +fn select_dll_layout(dll_layout: D, exe_layout: E) -> Result, UnpackError> +where + D: FnOnce() -> Result, UnpackError>, + E: FnOnce() -> Result, UnpackError>, +{ + match validate_dll_layout(dll_layout()) { + Ok(output) => Ok(output), + Err(dll) => validate_dll_layout(exe_layout()).map_err(|exe| { + UnpackError::PipelineFallbackFailed { + dll: Box::new(dll), + exe: Box::new(exe), + } + }), + } +} + +fn validate_dll_layout(output: Result, UnpackError>) -> Result, UnpackError> { + let output = output?; + if let Some(report) = dll_layout_invalid(&output) { + return Err(UnpackError::InvalidLayoutCandidate { + issues: report.findings.into_iter() + .filter(|finding| finding.severity == integrity::IntegritySeverity::Fatal) + .map(|finding| finding.message) + .collect(), + }); + } + Ok(output) +} + +/// Structural defects that mean a DLL layout candidate must not be accepted. +/// +/// Returns `None` when [`integrity::check`] reports nothing fatal. Warning +/// findings stay on the image: a native DLL with no base-relocation directory, +/// a zeroed or `int3` entry stub, or an import name that is mapped but not +/// printable ASCII. Those are the existing suspect conditions. A fatal finding +/// is one the loader or CLR rejects before that — the image is not a PE, a +/// claimed directory or entry point does not map, a section's raw range runs +/// past the file, or managed metadata has no COR20/BSJB signature. +/// +/// Severity comes from [`IntegrityReport::has_fatal`], which the checker sets +/// when it records each finding. This does not re-read issue text. +fn dll_layout_invalid(image: &[u8]) -> Option { + let report = integrity::check(image); + report.has_fatal().then_some(report) +} + #[cfg(test)] mod tests { use super::*; @@ -389,3 +430,303 @@ mod tests { assert!(column > 0); } } + +#[cfg(test)] +mod dll_layout_selection { + use super::*; + + fn pipeline_err(stage: &'static str) -> UnpackError { + UnpackError::InputTooShort { + actual: stage.len(), + required: 1, + } + } + + /// Minimal PE32+ image large enough for `integrity::check` to walk headers, + /// one section, and (when asked) an import name or a COR20 header. + struct SyntheticPe { + dll: bool, + size_of_image: u32, + entry_rva: u32, + entry_bytes: Vec, + executable: bool, + reloc: bool, + import_name: Option<&'static str>, + import_unmapped: bool, + managed: bool, + cor20_cb: u32, + metadata_sig: &'static [u8], + section_count: u16, + raw_past_eof: bool, + } + + impl Default for SyntheticPe { + fn default() -> Self { + Self { + dll: true, + size_of_image: 0x2000, + entry_rva: 0x1000, + entry_bytes: vec![0xC3], + executable: true, + reloc: true, + import_name: None, + import_unmapped: false, + managed: false, + cor20_cb: 0x48, + metadata_sig: b"BSJB", + section_count: 1, + raw_past_eof: false, + } + } + } + + impl SyntheticPe { + fn build(&self) -> Vec { + let optional_size: u16 = 0xF0; + let pe = 0x80usize; + let section = pe + 24 + optional_size as usize; + let file_size = if self.raw_past_eof { 0x200 } else { 0x500 }; + let mut data = vec![0u8; file_size]; + data[..2].copy_from_slice(b"MZ"); + data[0x3C..0x40].copy_from_slice(&(pe as u32).to_le_bytes()); + data[pe..pe + 4].copy_from_slice(b"PE\0\0"); + data[pe + 6..pe + 8].copy_from_slice(&self.section_count.to_le_bytes()); + let chars: u16 = if self.dll { 0x2000 } else { 0 }; + data[pe + 22..pe + 24].copy_from_slice(&chars.to_le_bytes()); + data[pe + 20..pe + 22].copy_from_slice(&optional_size.to_le_bytes()); + data[pe + 24..pe + 26].copy_from_slice(&0x20Bu16.to_le_bytes()); + data[pe + 24 + 16..pe + 24 + 20].copy_from_slice(&self.entry_rva.to_le_bytes()); + data[pe + 24 + 56..pe + 24 + 60].copy_from_slice(&self.size_of_image.to_le_bytes()); + // NumberOfRvaAndSizes must cover the directories we publish. + data[pe + 24 + 108..pe + 24 + 112].copy_from_slice(&16u32.to_le_bytes()); + let dd = pe + 24 + 112; + if self.reloc && !self.managed { + data[dd + 5 * 8..dd + 5 * 8 + 4].copy_from_slice(&0x1800u32.to_le_bytes()); + data[dd + 5 * 8 + 4..dd + 5 * 8 + 8].copy_from_slice(&8u32.to_le_bytes()); + } + if self.import_name.is_some() || self.import_unmapped { + data[dd + 8..dd + 12].copy_from_slice(&0x1100u32.to_le_bytes()); + data[dd + 12..dd + 16].copy_from_slice(&40u32.to_le_bytes()); + } + if self.managed { + data[dd + 14 * 8..dd + 14 * 8 + 4].copy_from_slice(&0x1400u32.to_le_bytes()); + data[dd + 14 * 8 + 4..dd + 14 * 8 + 8].copy_from_slice(&0x48u32.to_le_bytes()); + } + if self.section_count > 0 && section + 40 <= data.len() { + let characteristics: u32 = if self.executable { + 0x6000_0020 + } else { + 0x4000_0040 + }; + let raw_size: u32 = if self.raw_past_eof { 0x1000 } else { 0x300 }; + for (offset, value) in [ + (8usize, 0x1000u32), + (12, 0x1000), + (16, raw_size), + (20, 0x200), + (36, characteristics), + ] { + data[section + offset..section + offset + 4] + .copy_from_slice(&value.to_le_bytes()); + } + } + if self.entry_rva != 0 && !self.entry_bytes.is_empty() { + let at = 0x200 + (self.entry_rva.saturating_sub(0x1000)) as usize; + if let Some(slot) = data.get_mut(at..at + self.entry_bytes.len()) { + slot.copy_from_slice(&self.entry_bytes); + } + } + if self.import_name.is_some() || self.import_unmapped { + // Descriptor at RVA 0x1100 -> file 0x300. The name has to sit + // *before* that descriptor: bytes after it are the next + // descriptor, and a name like "kernel32.dll" would be walked + // as one. Mapped name RVA 0x1010 -> file 0x210, which is past + // the 16-byte entry stub and still inside the section. + let name_rva: u32 = if self.import_unmapped { 0x9000 } else { 0x1010 }; + if data.len() >= 0x300 + 20 { + data[0x30C..0x310].copy_from_slice(&name_rva.to_le_bytes()); + } + if let Some(name) = self.import_name.filter(|_| !self.import_unmapped) { + let name_at = 0x210; + let bytes = name.as_bytes(); + if let Some(slot) = data.get_mut(name_at..name_at + bytes.len()) { + slot.copy_from_slice(bytes); + } + } + } + if self.managed { + let cor = 0x200 + (0x1400 - 0x1000); + if data.len() >= cor + 0x48 { + data[cor..cor + 4].copy_from_slice(&self.cor20_cb.to_le_bytes()); + let md_rva = 0x1500u32; + data[cor + 8..cor + 12].copy_from_slice(&md_rva.to_le_bytes()); + data[cor + 12..cor + 16].copy_from_slice(&4u32.to_le_bytes()); + let md = 0x200 + (0x1500 - 0x1000); + let sig = self.metadata_sig; + if let Some(slot) = data.get_mut(md..md + sig.len()) { + slot.copy_from_slice(sig); + } + } + } + data + } + } + + fn valid_native_dll() -> Vec { + SyntheticPe::default().build() + } + + #[test] + fn invalid_dll_layout_falls_back_to_valid_exe_layout() { + let dll_image = SyntheticPe { + entry_rva: 0x9000, + ..SyntheticPe::default() + } + .build(); + let exe_image = valid_native_dll(); + let selected = select_dll_layout(|| Ok(dll_image.clone()), || Ok(exe_image.clone())) + .expect("valid EXE-layout image must be accepted"); + assert_eq!(selected, exe_image); + assert!(dll_layout_invalid(&dll_image).is_some()); + assert!(dll_layout_invalid(&exe_image).is_none()); + } + + #[test] + fn structurally_valid_dll_layout_is_kept_without_calling_exe() { + let dll_image = valid_native_dll(); + let selected = select_dll_layout( + || Ok(dll_image.clone()), + || panic!("EXE layout must not run when the DLL layout is valid"), + ) + .expect("DLL layout"); + assert_eq!(selected, dll_image); + } + + #[test] + fn dll_warning_is_not_fatal_and_blocks_exe_fallback() { + let warned = SyntheticPe { + reloc: false, + ..SyntheticPe::default() + } + .build(); + let report = integrity::check(&warned); + assert!(!report.ok(), "fixture must actually warn, got {report:?}"); + assert!( + dll_layout_invalid(&warned).is_none(), + "a missing relocation directory is a warning, not a rejected layout" + ); + let selected = select_dll_layout( + || Ok(warned.clone()), + || panic!("warnings must not trigger the EXE layout"), + ) + .expect("warned DLL layout"); + assert_eq!(selected, warned); + } + + #[test] + fn zeroed_entry_stub_is_a_warning_not_a_rejected_layout() { + let warned = SyntheticPe { + entry_bytes: vec![0; 16], + ..SyntheticPe::default() + } + .build(); + let report = integrity::check(&warned); + assert!( + report + .issues + .iter() + .any(|issue| issue.contains("all zeros")) + ); + assert!(dll_layout_invalid(&warned).is_none()); + } + + #[test] + fn both_layouts_structurally_invalid_reports_both_candidates() { + let invalid = SyntheticPe { + entry_rva: 0x9000, + ..SyntheticPe::default() + } + .build(); + let error = select_dll_layout(|| Ok(invalid.clone()), || Ok(invalid.clone())) + .expect_err("neither layout is acceptable"); + match error { + UnpackError::PipelineFallbackFailed { dll, exe } => { + let dll = dll.to_string(); + let exe = exe.to_string(); + assert!(dll.contains("structurally invalid"), "{dll}"); + assert!(exe.contains("structurally invalid"), "{exe}"); + } + other => panic!("unexpected error: {other}"), + } + } + + #[test] + fn dll_error_then_exe_error_keeps_pipeline_diagnostics() { + let error = select_dll_layout( + || Err(pipeline_err("dll")), + || Err(pipeline_err("exe-layout")), + ) + .expect_err("both pipelines failed"); + match error { + UnpackError::PipelineFallbackFailed { dll, exe } => { + let dll = dll.to_string(); + let exe = exe.to_string(); + assert!(dll.contains("got 3"), "{dll}"); + assert!(exe.contains("got 10"), "{exe}"); + } + other => panic!("unexpected error: {other}"), + } + } + + #[test] + fn dll_error_then_invalid_exe_image_names_both_failures() { + let invalid = SyntheticPe { + raw_past_eof: true, + ..SyntheticPe::default() + } + .build(); + assert!(dll_layout_invalid(&invalid).is_some()); + let error = select_dll_layout(|| Err(pipeline_err("dll")), || Ok(invalid)) + .expect_err("EXE image is not a usable fallback"); + match error { + UnpackError::PipelineFallbackFailed { dll, exe } => { + let dll = dll.to_string(); + let exe = exe.to_string(); + assert!(dll.contains("need at least 1"), "{dll}"); + assert!(exe.contains("structurally invalid"), "{exe}"); + } + other => panic!("unexpected error: {other}"), + } + } + + #[test] + fn unmapped_import_and_bad_metadata_are_fatal() { + let imports = SyntheticPe { + import_unmapped: true, + import_name: Some("unused"), + ..SyntheticPe::default() + } + .build(); + assert!(dll_layout_invalid(&imports).is_some()); + + let metadata = SyntheticPe { + managed: true, + reloc: false, + metadata_sig: b"XXXX", + ..SyntheticPe::default() + } + .build(); + assert!(dll_layout_invalid(&metadata).is_some()); + + let readable = SyntheticPe { + import_name: Some("kernel32.dll"), + ..SyntheticPe::default() + } + .build(); + assert!( + dll_layout_invalid(&readable).is_none(), + "{:?}", + integrity::check(&readable).issues + ); + } +}