nop: Fix off-by-one in unmap check (#960)

hugsy · Jul 14, 2023 · ca7418c · ca7418c
1 parent 74e8626
commit ca7418c
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 10 deletions.
diff --git a/gef.py b/gef.py
@@ -6030,9 +6030,9 @@ def do_invoke(self, _: List[str], **kwargs: Any) -> None:
 
         if total_bytes % len(nop):
             warn(f"Patching {total_bytes} bytes at {address:#x} will result in a partially patched instruction and may break disassembly")
-        
+
         nops = bytearray(nop * (total_bytes // len(nop)))
-        end_address = Address(value=address + total_bytes)
+        end_address = Address(value=address + total_bytes - 1)
         if not end_address.valid:
             err(f"Cannot patch instruction at {address:#x}: reaching unmapped area")
             return

diff --git a/tests/commands/nop.py b/tests/commands/nop.py
@@ -21,7 +21,7 @@ def test_cmd_nop_inactive(self):
 
     @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
     def test_cmd_nop_no_arg(self):
-        
+
         res = gdb_start_silent_cmd(
             "pi gef.memory.write(gef.arch.pc, p32(0xfeebfeeb))", # 2 short jumps to pc
             after=(
@@ -30,12 +30,12 @@ def test_cmd_nop_no_arg(self):
             )
         )
         self.assertNoException(res)
-        self.assertIn(r'\x90\x90\xeb\xfe', res) # 2 nops + 1 short jump
+        self.assertIn(r"\x90\x90\xeb\xfe", res) # 2 nops + 1 short jump
 
 
     @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
     def test_cmd_nop_arg(self):
-        
+
         res = gdb_start_silent_cmd(
             "pi gef.memory.write(gef.arch.sp, p64(0xfeebfeebfeebfeeb))",  # 4 short jumps to stack
             after=(
@@ -44,7 +44,7 @@ def test_cmd_nop_arg(self):
             )
         )
         self.assertNoException(res)
-        self.assertIn(r'\x90\x90\x90\x90\xeb\xfe\xeb\xfe', res) #  4 nops + 2 short jumps
+        self.assertIn(r"\x90\x90\x90\x90\xeb\xfe\xeb\xfe", res) #  4 nops + 2 short jumps
 
 
     @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
@@ -63,13 +63,13 @@ def test_cmd_nop_as_bytes_no_arg(self):
             "pi print(f'*** *pc={u8(gef.memory.read(gef.arch.pc, 1))}')",
             after=(
                 f"{self.cmd} --b",
-                "pi print(f'*** *pc={u8(gef.memory.read(gef.arch.pc, 1))}')",
+                "pi print(f'*** *pc={u8(gef.memory.read(gef.arch.pc, 1)):#x}')",
             )
         )
         self.assertNoException(res)
         lines = findlines("*** *pc=", res)
         self.assertEqual(len(lines), 2)
-        self.assertEqual(lines[1], "*** *pc=144") # nop -> 0x90 -> 144
+        self.assertEqual(lines[1], "*** *pc=0x90")
 
 
     @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
@@ -78,20 +78,34 @@ def test_cmd_nop_as_bytes_arg(self):
             "pi print(f'*** *sp={u32(gef.memory.read(gef.arch.sp, 4))}')",
             after=(
                 f"{self.cmd} --b --n 4 $sp",
-                "pi print(f'*** *sp={u32(gef.memory.read(gef.arch.sp, 4))}')",
+                "pi print(f'*** *sp={u32(gef.memory.read(gef.arch.sp, 4)):#x}')",
             )
         )
         self.assertNoException(res)
         lines = findlines("*** *sp=", res)
         self.assertEqual(len(lines), 2)
-        self.assertEqual(lines[1], "*** *sp=2425393296") # 4*nop -> 0x90909090 -> 2425393296
+        self.assertEqual(lines[1], "*** *sp=0x90909090")
 
 
     @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
     def test_cmd_nop_as_bytes_invalid_end_address(self):
+        # Make sure we error out if writing nops into an unmapped or RO area
         res = gdb_run_silent_cmd(
             f"{self.cmd} --b --n 5 0x1337000+0x1000-4",
             target=_target("mmap-known-address")
         )
         self.assertNoException(res)
         self.assertIn("Cannot patch instruction at 0x1337ffc: reaching unmapped area", res)
+
+        # We had an off-by-one bug where we couldn't write the last byte before
+        # an unmapped area. Make sure that we can now.
+        res = gdb_run_silent_cmd(
+            f"{self.cmd} --b --n 4 0x1337000+0x1000-4",
+            target=_target("mmap-known-address"),
+            after="pi print(f'*** *mem={u32(gef.memory.read(0x1337ffc, 4)):#x}')",
+        )
+        self.assertNoException(res)
+        self.assertNotIn("Cannot patch instruction at 0x1337ffc: reaching unmapped area", res)
+        lines = findlines("*** *mem=", res)
+        self.assertEqual(len(lines), 1)
+        self.assertEqual(lines[0], "*** *mem=0x90909090")