add new nop command features #967

therealdreg · 2023-07-20T18:10:13Z

Description/Motivation/Screenshots

The `nop` command allows you to easily patch instructions with nops.

nop [LOCATION] [--i ITEMS] [--f] [--n] [--b]


`LOCATION` address/symbol to patch (by default this command replaces whole instructions)

`--i ITEMS` number of items to insert (default 1)

`--f` Force patch even when the selected settings could overwrite partial instructions

`--n` Instead of replacing whole instructions, insert ITEMS nop instructions, no matter how many instructions it overwrites

`--b` Instead of replacing whole instructions, fill ITEMS bytes with nops

gef➤ 	nop
gef➤ 	nop $pc+3
gef➤ 	nop --i 2 $pc+3
gef➤ 	nop --b
gef➤ 	nop --b $pc+3
gef➤ 	nop --f --b --i 2 $pc+3
gef➤ 	nop --n --i 2 $pc+3

Against which architecture was this tested ?

"Tested" indicates that the PR works and the unit test (see docs/testing.md) run passes without issue.

Checklist

My PR was done against the dev branch, not main.
My code follows the code style of this project.
My change includes a change to the documentation, if required.
If my change adds new code, adequate tests have been added.
I have read and agree to the CONTRIBUTING document.

Grazfather · 2023-07-20T18:24:13Z

docs/commands/nop.md

 ```

-`LOCATION` address/symbol to patch
+`LOCATION` address/symbol to patch (by default this command replace whole instruction(s))


Suggested change

`LOCATION` address/symbol to patch (by default this command replace whole instruction(s))

`LOCATION` address/symbol to patch (by default this command replaces whole instructions)

Grazfather · 2023-07-20T18:25:21Z

docs/commands/nop.md

+
+`--f` Force patch when the final instruction/nop can be broken
+
+`--n` Instead replace whole instruction(s), insert the number specified by ITEMS-value of nop(s)-instruction(s)


Suggested change

`--n` Instead replace whole instruction(s), insert the number specified by ITEMS-value of nop(s)-instruction(s)

`--n` Instead of replacing whole instructions, insert ITEMS nop instructions, no matter how many instructions it overwrites

Grazfather · 2023-07-20T18:26:00Z

docs/commands/nop.md

+
+`--n` Instead replace whole instruction(s), insert the number specified by ITEMS-value of nop(s)-instruction(s)
+
+`--b` Instead replace whole instruction(s), fill with nop(s)-instruction(s) the number specified by ITEMS-value bytes


Suggested change

`--b` Instead replace whole instruction(s), fill with nop(s)-instruction(s) the number specified by ITEMS-value bytes

`--b` Instead of replacing whole instructions, fill ITEMS bytes with nops

Grazfather · 2023-07-20T18:27:11Z

docs/commands/nop.md

-instructions/nops (full instruction size by default)
+`--i ITEMS` number of items to insert (default 1)
+
+`--f` Force patch when the final instruction/nop can be broken


Suggested change

`--f` Force patch when the final instruction/nop can be broken

`--f` Force patch even when the selected settings could overwrite partial instructions

Grazfather · 2023-07-20T18:27:27Z

gef.py

+ "\n\tLOCATION\taddress/symbol to patch (by default this command replace whole instruction(s))"
+ "\t--i ITEMS\tnumber of items to insert (default 1)"
+ "\t--f\tForce patch when the final instruction/nop can be broken"
+ "\t--n\tInstead replace whole instruction(s), insert the number specified by ITEMS-value of nop(s)-instruction(s)"


take from my suggestions above

Grazfather · 2023-07-20T18:29:12Z

gef.py

+ force_flag = args.f or False
+ fill_instructions = False if fill_nops or fill_bytes else True
+
+ if fill_nops + fill_bytes + fill_instructions != 1:


Suggested change

if fill_nops + fill_bytes + fill_instructions != 1:

if fill_nops and fill_bytes:

gef.py

Grazfather · 2023-07-20T18:29:50Z

gef.py

- 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")
+ if len(nop) > total_bytes or total_bytes % len(nop):
+ warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-NOP (byte nr {total_bytes % len(nop):#x}) broken and may cause a crash or break disassembly")


please break up long lines

Grazfather · 2023-07-20T18:30:31Z

gef.py

+ if len(nop) > total_bytes or total_bytes % len(nop):
+ warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-NOP (byte nr {total_bytes % len(nop):#x}) broken and may cause a crash or break disassembly")
+ if not force_flag:
+ err("you must use --f to allow this kind of patch")


Suggested change

err("you must use --f to allow this kind of patch")

err("Use --f (force) to ignore this warning")

Grazfather · 2023-07-20T18:31:13Z

gef.py

+ final_ins_end_addr = curr_ins.address + curr_ins.size()
+
+ if final_ins_end_addr != target_end_address:
+ warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-INSTRUCTION ({curr_ins.address:#x}) broken and may cause a crash or break disassembly")


Suggested change

warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-INSTRUCTION ({curr_ins.address:#x}) broken and may cause a crash or break disassembly")

warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-INSTRUCTION ({curr_ins.address:#x}) being partial overwritten and may cause a crash or break disassembly")

and also wrap

hugsy

a few things to changes (also from graz's review) then we're good

gef.py

hugsy · 2023-07-20T21:47:48Z

gef.py

+ if final_ins_end_addr != target_end_address:
+ warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-INSTRUCTION ({curr_ins.address:#x}) broken and may cause a crash or break disassembly")
+ if not force_flag:
+ err("you must use --f to allow this kind of patch")


Suggested change

err("you must use --f to allow this kind of patch")

err("You must use --f to allow misaligned patching.")

hugsy · 2023-07-20T21:48:32Z

gef.py

+ err("you must use --f to allow this kind of patch")
+ return
+
+ nops = bytearray(nop * total_bytes) # this array will be bigger than needed when arch nop is > 1 but who cares


Comment is unhelpful, remove

therealdreg · 2023-07-21T00:24:16Z

btw, @hugsy suggests in discord join the "warning msg" + "use --f msg": done

hugsy

I think examples in the docs need a bit more explanation in plain english.

docs/commands/nop.md

Grazfather · 2023-07-21T16:19:33Z

docs/commands/nop.md


+`--n` Instead of replacing whole instructions, insert ITEMS nop instructions, no matter how many instructions it overwrites


wrap to 100 according to editorconfig

I don't think it is?

docs/commands/nop.md

Grazfather · 2023-07-21T16:20:56Z

docs/commands/nop.md

+gef➤ nop --f --b --i 2 $pc+3
+```
+
+Patching with 2 nops at $pc+3 address:


Patch 2 nops at address $pc+3

Grazfather · 2023-07-21T16:21:49Z

gef.py


 def __init__(self) -> None:
 super().__init__(complete=gdb.COMPLETE_LOCATION)
 return

 @only_if_gdb_running
- @parse_arguments({"address": "$pc"}, {"--n": 0, "--b": False})
+ @parse_arguments({"address": "$pc"}, {"--i": 1, "--b": True, "--f": True, "--n": True})


but how can that work?

gef.py

Grazfather · 2023-07-21T16:25:05Z

gef.py

+ target_end_address = address + total_bytes
+ curr_ins = gef_current_instruction(address)
+ while curr_ins.address + curr_ins.size() < target_end_address:
+ if not Address(value=curr_ins.address + 1).valid:


the +1 is confusing. Why not but this check after curr_ins = get next so you don't need the +1?

isn't curr_ins.address already an Address? if so, if not curr_ins.valid

because I dont want disasm invalid Address (avoid exception / error .. etc) I think the current code is OK and more secure

gef.py

therealdreg · 2023-07-21T19:33:40Z

added a test to check if --b and --n at same time == error

hugsy · 2023-07-21T19:51:00Z

gef.py

+ if final_ins_end_addr != target_end_address:
+ warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-INSTRUCTION "
+ f"({curr_ins.address:#x}) being partial overwritten and may cause a crash or "
+ f"break disassembly. You must use --f to allow misaligned patching.")


Suggested change

f"break disassembly. You must use --f to allow misaligned patching.")

"break disassembly. You must use --f to allow misaligned patching.")

Grazfather reviewed Jul 20, 2023

View reviewed changes

hugsy requested changes Jul 20, 2023

View reviewed changes

therealdreg requested review from Grazfather and hugsy July 21, 2023 00:28

hugsy requested changes Jul 21, 2023

View reviewed changes

docs/commands/nop.md Show resolved Hide resolved

therealdreg requested a review from hugsy July 21, 2023 06:42

hugsy approved these changes Jul 21, 2023

View reviewed changes

Grazfather reviewed Jul 21, 2023

View reviewed changes

therealdreg changed the title ~~new nop command features~~ add new nop command features Jul 21, 2023

therealdreg requested review from Grazfather and hugsy July 21, 2023 19:15

add new nop command features

22b4a28

hugsy approved these changes Jul 21, 2023

View reviewed changes

hugsy merged commit 99c59a9 into hugsy:dev Jul 21, 2023
3 checks passed

hugsy added this to the Release: next milestone Jul 21, 2023

	`LOCATION` address/symbol to patch (by default this command replace whole instruction(s))
	`LOCATION` address/symbol to patch (by default this command replaces whole instructions)


		`--f` Force patch when the final instruction/nop can be broken

		`--n` Instead replace whole instruction(s), insert the number specified by ITEMS-value of nop(s)-instruction(s)

	`--n` Instead replace whole instruction(s), insert the number specified by ITEMS-value of nop(s)-instruction(s)
	`--n` Instead of replacing whole instructions, insert ITEMS nop instructions, no matter how many instructions it overwrites


		`--n` Instead replace whole instruction(s), insert the number specified by ITEMS-value of nop(s)-instruction(s)

		`--b` Instead replace whole instruction(s), fill with nop(s)-instruction(s) the number specified by ITEMS-value bytes

	`--b` Instead replace whole instruction(s), fill with nop(s)-instruction(s) the number specified by ITEMS-value bytes
	`--b` Instead of replacing whole instructions, fill ITEMS bytes with nops

	`--f` Force patch when the final instruction/nop can be broken
	`--f` Force patch even when the selected settings could overwrite partial instructions

	if fill_nops + fill_bytes + fill_instructions != 1:
	if fill_nops and fill_bytes:

	err("you must use --f to allow this kind of patch")
	err("Use --f (force) to ignore this warning")

	warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-INSTRUCTION ({curr_ins.address:#x}) broken and may cause a crash or break disassembly")
	warn(f"Patching {total_bytes} bytes at {address:#x} will result in LAST-INSTRUCTION ({curr_ins.address:#x}) being partial overwritten and may cause a crash or break disassembly")

	err("you must use --f to allow this kind of patch")
	err("You must use --f to allow misaligned patching.")

	f"break disassembly. You must use --f to allow misaligned patching.")
	"break disassembly. You must use --f to allow misaligned patching.")

add new nop command features #967

add new nop command features #967

Conversation

therealdreg commented Jul 20, 2023 • edited Loading

Description/Motivation/Screenshots

Against which architecture was this tested ?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugsy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

therealdreg commented Jul 21, 2023 • edited Loading

hugsy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

therealdreg Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

therealdreg commented Jul 21, 2023

Choose a reason for hiding this comment

therealdreg commented Jul 20, 2023 •

edited

Loading

therealdreg commented Jul 21, 2023 •

edited

Loading

therealdreg Jul 21, 2023 •

edited

Loading