Fault on zero page memory addresses

[encounter]

Right now, retrowin32 allows reads and writes to memory addresses < 0x1000 without complaint. This can hide or obfuscate bugs that would normally result in crashes. This affects both x86-emu and x86-unicorn.

This is definitely not the right approach, but here's what I did in order to track down invalid memory r/w using x86-emu:

diff --git a/x86/src/ops/helpers.rs b/x86/src/ops/helpers.rs
--- a/x86/src/ops/helpers.rs	(revision 5b59c5ea357eda228c88fd3a882854aa4889ea31)
+++ b/x86/src/ops/helpers.rs	(date 1721951565234)
@@ -18,7 +18,11 @@
             cpu.regs.set64(reg, value);
         }
         iced_x86::OpKind::Memory => {
-            let addr = x86_addr(cpu, instr);
+            let mut addr = x86_addr(cpu, instr);
+            if mem.is_oob::<u64>(addr) {
+                cpu.err(format!("oob at {addr:x}"));
+                addr = 0;
+            }
             let x = mem.get_pod::<u64>(addr);
             let value = op(cpu, x);
             mem.put::<u64>(addr, value);
@@ -55,7 +59,11 @@
             Arg(cpu.regs.get32_mut(reg))
         }
         iced_x86::OpKind::Memory => {
-            let addr = x86_addr(cpu, instr);
+            let mut addr = x86_addr(cpu, instr);
+            if mem.is_oob::<u32>(addr) {
+                cpu.err(format!("oob at {addr:x}"));
+                addr = 0;
+            }
             Arg(mem.ptr_mut::<u32>(addr))
         }
         _ => unimplemented!(),
@@ -69,7 +77,11 @@
             Arg(cpu.regs.get16_mut(reg))
         }
         iced_x86::OpKind::Memory => {
-            let addr = x86_addr(cpu, instr);
+            let mut addr = x86_addr(cpu, instr);
+            if mem.is_oob::<u16>(addr) {
+                cpu.err(format!("oob at {addr:x}"));
+                addr = 0;
+            }
             Arg(mem.ptr_mut::<u16>(addr))
         }
         _ => unimplemented!(),
@@ -112,7 +124,14 @@
 pub fn op1_rm16(cpu: &mut CPU, mem: Mem, instr: &iced_x86::Instruction) -> u16 {
     match instr.op1_kind() {
         iced_x86::OpKind::Register => cpu.regs.get16(instr.op1_register()),
-        iced_x86::OpKind::Memory => mem.get_pod::<u16>(x86_addr(cpu, instr)),
+        iced_x86::OpKind::Memory => {
+            let addr = x86_addr(cpu, instr);
+            if mem.is_oob::<u16>(addr) {
+                cpu.err(format!("oob at {addr:x}"));
+                return 0;
+            }
+            mem.get_pod::<u16>(addr)
+        },
         _ => unreachable!(),
     }
 }
@@ -120,7 +139,14 @@
 pub fn op1_rm8(cpu: &mut CPU, mem: Mem, instr: &iced_x86::Instruction) -> u8 {
     match instr.op1_kind() {
         iced_x86::OpKind::Register => cpu.regs.get8(instr.op1_register()),
-        iced_x86::OpKind::Memory => mem.get_pod::<u8>(x86_addr(cpu, instr)),
+        iced_x86::OpKind::Memory => {
+            let addr = x86_addr(cpu, instr);
+            if mem.is_oob::<u8>(addr) {
+                cpu.err(format!("oob at {addr:x}"));
+                return 0;
+            }
+            mem.get_pod::<u8>(addr)
+        },
         _ => unreachable!(),
     }
 }
Index: memory/src/mem.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/memory/src/mem.rs b/memory/src/mem.rs
--- a/memory/src/mem.rs	(revision 5b59c5ea357eda228c88fd3a882854aa4889ea31)
+++ b/memory/src/mem.rs	(date 1721951565274)
@@ -96,6 +96,9 @@
     }
 
     pub fn is_oob<T>(&self, addr: u32) -> bool {
+        if addr < 0x1000 {
+            return true;
+        }
         self.ptr as usize + addr as usize + size_of::<T>() > self.end as usize
     }
 

[evmar]

One reason I couldn't make Mem blanket disallow accesses <0x1000 is that we currently use sub-slices of Mem where accesses <0x1000 are valid. This is another issue that might be helped by the idea in #26 , by making it so it's always an error for Mem to access <0x1000 as you have in is_oob above.

[evmar]

After some of the fixes from #26 I tried this, which makes anything that creates a pointer <4k crash, and it successfully reproduced the issue in #28 while still letting BasicDD work.

--- a/memory/src/mem.rs
+++ b/memory/src/mem.rs
@@ -117,7 +117,7 @@ impl<'m> Mem<'m> {
     }
 
     pub fn is_oob<T>(&self, addr: u32) -> bool {
-        self.ptr as usize + addr as usize + size_of::<T>() > self.end as usize
+        addr < 0x1000 || self.ptr as usize + addr as usize + size_of::<T>() > self.end as usize
     }

[evmar]

New blockers are functions like

retrowin32/win32/src/winapi/ucrtbase.rs

Line 49 in ce12d36

pub fn __p___argv(_machine: &mut Machine) -> u32 {

which return null ptrs that get dereferenced in crt startup.