Update nop command to patch entire instructions (#959)

* The default behavior to `nop` is to patch entire instruction(s) unless indicated otherwise via the `--b` toggle
* `--n` can be used to specify the number of nops to insert

docs/commands/nop.md

@@ -1,18 +1,22 @@
 ## Command `nop`
 
-The `nop` command allows you to easily skip instructions.
+The `nop` command allows you to easily patch instructions with nops.
 
 ```
-gef ➤  help nop
-Patch the instruction(s) pointed by parameters with NOP. Note: this command is architecture
-aware.
-Syntax: nop [LOCATION] [--nb NUM_BYTES]
-  LOCATION      address/symbol to patch
-    --nb NUM_BYTES      Instead of writing one instruction, patch the specified number of bytes
+nop [LOCATION] [--n NUM_ITEMS] [--b]
 ```
 
-`LOCATION` indicates the address of the instruction to bypass. If not
-specified, it will use the current value of the program counter.
+`LOCATION` address/symbol to patch	
 
-If `--nb <bytes>` is entered, gef will explicitly patch the specified number of
-bytes.  Otherwise it will patch the _whole_ instruction at the target location.
+`--n NUM_ITEMS` Instead of writing one instruction/nop, patch the specified number of instructions/nops (full instruction size by default)	
+
+`--b` Instead of writing full instruction size, patch the specified number of nops
+
+```bash
+gef➤ 	nop
+gef➤ 	nop $pc+3
+gef➤ 	nop --n 2 $pc+3
+gef➤ 	nop --b
+gef➤ 	nop --b $pc+3
+gef➤ 	nop --b --n 2 $pc+3
+```

gef.py

@@ -5991,35 +5991,54 @@ class NopCommand(GenericCommand):
     aware."""
 
     _cmdline_ = "nop"
-    _syntax_  = ("{_cmdline_} [LOCATION] [--nb NUM_BYTES]"
+    _syntax_  = ("{_cmdline_} [LOCATION] [--n NUM_ITEMS] [--b]"
                  "\n\tLOCATION\taddress/symbol to patch"
-                 "\t--nb NUM_BYTES\tInstead of writing one instruction, patch the specified number of bytes")
+                 "\t--n NUM_ITEMS\tInstead of writing one instruction/nop, patch the specified number of instructions/nops (full instruction size by default)"
+                 "\t--b\tInstead of writing full instruction size, patch the specified number of nops")
     _example_ = f"{_cmdline_} $pc"
 
+    _example_ = [f"{_cmdline_}",
+                 f"{_cmdline_} $pc+3",
+                 f"{_cmdline_} --n 2 $pc+3",
+                 f"{_cmdline_} --b",
+                 f"{_cmdline_} --b $pc+3",
+                 f"{_cmdline_} --b --n 2 $pc+3",]
+
     def __init__(self) -> None:
         super().__init__(complete=gdb.COMPLETE_LOCATION)
         return
 
     @only_if_gdb_running
-    @parse_arguments({"address": "$pc"}, {"--nb": 0, })
+    @parse_arguments({"address": "$pc"}, {"--n": 0, "--b": False})
     def do_invoke(self, _: List[str], **kwargs: Any) -> None:
         args : argparse.Namespace = kwargs["arguments"]
         address = parse_address(args.address)
         nop = gef.arch.nop_insn
-        number_of_bytes = args.nb or 1
-        insn = gef_get_instruction_at(address)
+        num_items = args.n or 1
+        as_nops_flags = not args.b
 
-        if insn.size() != number_of_bytes:
-            warn(f"Patching {number_of_bytes} bytes at {address:#x} might result in corruption")
+        total_bytes = 0
+        if as_nops_flags:
+            total_bytes = num_items * len(nop)
+        else:
+            try:
+                last_addr = gdb_get_nth_next_instruction_address(address, num_items)
+            except:
+                err(f"Cannot patch instruction at {address:#x}: MAYBE reaching unmapped area")
+                return
+            total_bytes = (last_addr - address) + gef_get_instruction_at(last_addr).size()
 
-        nops = bytearray(nop * number_of_bytes)
-        end_address = Address(value=address + len(nops))
+        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)
         if not end_address.valid:
             err(f"Cannot patch instruction at {address:#x}: reaching unmapped area")
             return
 
-        ok(f"Patching {len(nops)} bytes from {address:#x}")
-        gef.memory.write(address, nops, len(nops))
+        ok(f"Patching {total_bytes} bytes from {address:#x}")
+        gef.memory.write(address, nops, total_bytes)
         return
 
 

tests/commands/nop.py

@@ -19,13 +19,50 @@ def test_cmd_nop_inactive(self):
         res = gdb_run_cmd(f"{self.cmd}")
         self.assertFailIfInactiveSession(res)
 
-
     @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 print(f'*** *pc={u8(gef.memory.read(gef.arch.pc, 1))}')",
+            "pi gef.memory.write(gef.arch.pc, p32(0xfeebfeeb))", # 2 short jumps to pc
             after=(
                 self.cmd,
+                "pi print(gef.memory.read(gef.arch.pc, 4))", # read 4 bytes
+            )
+        )
+        self.assertNoException(res)
+        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=(
+                f"{self.cmd} --n 2 $sp",
+                "pi print(gef.memory.read(gef.arch.sp, 8))",  # read 8 bytes
+            )
+        )
+        self.assertNoException(res)
+        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}")
+    def test_cmd_nop_invalid_end_address(self):
+        res = gdb_run_silent_cmd(
+            f"{self.cmd} --n 5 0x1337000+0x1000-4",
+            target=_target("mmap-known-address")
+        )
+        self.assertNoException(res)
+        self.assertIn("reaching unmapped area", res)
+
+
+    @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
+    def test_cmd_nop_as_bytes_no_arg(self):
+        res = gdb_start_silent_cmd(
+            "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))}')",
             )
         )
@@ -36,11 +73,11 @@ def test_cmd_nop_no_arg(self):
 
 
     @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
-    def test_cmd_nop_arg(self):
+    def test_cmd_nop_as_bytes_arg(self):
         res = gdb_start_silent_cmd(
             "pi print(f'*** *sp={u32(gef.memory.read(gef.arch.sp, 4))}')",
             after=(
-                f"{self.cmd} $sp --nb 4",
+                f"{self.cmd} --b --n 4 $sp",
                 "pi print(f'*** *sp={u32(gef.memory.read(gef.arch.sp, 4))}')",
             )
         )
@@ -51,9 +88,9 @@ def test_cmd_nop_arg(self):
 
 
     @pytest.mark.skipif(ARCH not in ("i686", "x86_64"), reason=f"Skipped for {ARCH}")
-    def test_cmd_nop_invalid_end_address(self):
+    def test_cmd_nop_as_bytes_invalid_end_address(self):
         res = gdb_run_silent_cmd(
-            f"{self.cmd} 0x1337000+0x1000-4 --nb 5",
+            f"{self.cmd} --b --n 5 0x1337000+0x1000-4",
             target=_target("mmap-known-address")
         )
         self.assertNoException(res)