modified mem_writer and mem_changer#144
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the memory writing functionality in angrop to automatically handle bad bytes by searching for and composing appropriate gadgets. Instead of simply failing when encountering bad bytes in data, the system can now transform bytes using memory modification operations (XOR, OR, AND, ADD) to write the target values without including bad bytes in the ROP chain payload.
Changes:
- Added sophisticated byte transformation algorithms in mem_writer.py that can find safe initial values and operations to achieve target bytes while avoiding bad bytes
- Fixed data size handling in mem_changer.py to properly mask initial values and use correct byte sizes for memory operations
- Added comprehensive tests to verify bad byte handling and correct memory operation size/endness parameters
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| angrop/chain_builder/mem_writer.py | Added new transformation methods (_byte_candidates, _solve_byte_pair, _find_chunk_transform, _plan_bytewise_fix, _has_mem_change_gadget) and rewrote write_to_mem to use chunk-based transformation approach |
| angrop/chain_builder/mem_changer.py | Fixed init_val masking and corrected memory store size parameters (using data_bytes instead of arch_bytes) |
| tests/test_chainbuilder.py | Added three new tests: test_write_to_mem_badbyte_transform, test_write_to_mem_badbyte_multibyte, and test_mem_changer_store_size_and_endness |
| .gitignore | Added "build" directory to gitignore |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for chunk_size in chunk_sizes: | ||
| if offset + chunk_size > data_len: | ||
| continue | ||
| ptr = addr + offset |
There was a problem hiding this comment.
we might want to add a check here so that we can plan ahead and avoid the self._word_contain_badbyte check being triggered in the next iteration. Essentially, if this is not the last iteration, check whether the next ptr contains badbyte, if yes, don't use this chunk_size. if none of the chunk_size works, raise.
There is a way to solve this planning problem properly. But let's not do it in the PR. So maybe you can add a TODO here about fixing this so that we can plan ahead and completely avoid having to deal with pointers containing bad bytes.
angrop/chain_builder/mem_writer.py
Outdated
| continue | ||
| if any(b in badbytes for b in arg_blob): | ||
| continue | ||
| endian = "little" if "LE" in str(self.project.arch.memory_endness) else "big" |
There was a problem hiding this comment.
let's use self.project.arch.memory_endness == "Iend_LE here instead of "LE" in str(self.project.arch.memory_endness)
angrop/chain_builder/mem_writer.py
Outdated
| seen.add(i) | ||
| yield i | ||
|
|
||
| def _find_single_byte_transform(self, target, badbytes, preferred_init=None): |
There was a problem hiding this comment.
can you remove this function since it is not being used?
|
Can you please rebase to master branch, address issues I mentioned, and fix testcases in the CI? |
angrop/chain_builder/mem_writer.py
Outdated
| inits = list(self._byte_candidates(preferred_init, badbytes)) | ||
| args = [x for x in range(0x100) if x not in badbytes] | ||
|
|
||
| for op, func in ops: |
There was a problem hiding this comment.
consider using itertools.product to cleanup the code
|
Overall, pretty cool and fast solution. And the way it works is exactly the way I wanted (users just do a |
|
Can you remove |
|
will put back test cases to test_badbytes.py. |
fixed allow_badbyte_addr, but add another logic for dealing with it |
|
Everything looks perfect now. Thanks a lot for the contribution! |
Changed bad bytes mem writing method.
Now angrop can automaticly search for proper gadgets for mem bytes modify and compose them together.