fix(windows): validate DLL layout candidates before selection

senbei-engine/src/windows/error.rs

@@ -224,6 +224,9 @@ pub enum UnpackError {
         exe: Box<UnpackError>,
     },
 
+    #[error("structurally invalid layout candidate: {}", issues.join("; "))]
+    InvalidLayoutCandidate { issues: Vec<String> },
+
     #[error(
         "PE32 second-stage range is invalid (offset {offset}, size {size}, image length {image_len})"
     )]

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<String>,
+    /// The same defects as `issues`, each tagged fatal or warning.
+    pub findings: Vec<IntegrityFinding>,
 }
 
 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<String>) {
+        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)"
+                                        ),
+                                    );
                                 }
                             }
                         }

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<u8>), 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<D, E>(dll_layout: D, exe_layout: E) -> Result<Vec<u8>, UnpackError>
+where
+    D: FnOnce() -> Result<Vec<u8>, UnpackError>,
+    E: FnOnce() -> Result<Vec<u8>, 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<Vec<u8>, UnpackError>) -> Result<Vec<u8>, 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<IntegrityReport> {
+    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<u8>,
+        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<u8> {
+            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<u8> {
+        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
+        );
+    }
+}