Fix x86 gadget discovery by decoupling alignment from step size#219
Merged
SweetVishnya merged 2 commits intoJonathanSalwan:masterfrom Nov 22, 2025
Merged
Fix x86 gadget discovery by decoupling alignment from step size#219SweetVishnya merged 2 commits intoJonathanSalwan:masterfrom
SweetVishnya merged 2 commits intoJonathanSalwan:masterfrom
Conversation
ROPgadget previously used the alignment value as the backward step size when scanning for gadget start offsets. This prevents discovery of valid x86 gadgets such as “pop rdi; ret”, when specifying certain alignment values (namely 2-byte alignment). This patch steps backward one byte at a time on all architectures and applies the alignment constraint only as a filter on the start address. This preserves correct behavior for fixed-width ISAs (ARM Thumb, RISC-V compressed) while restoring correct gadget discovery on x86/x64.
Collaborator
|
@iilegacyyii, could you look into the failing tests, please? |
Contributor
Author
|
Yep of course. Will take a look after work today. |
Reworked `__gadgetsFinding` to restore the original aligned-step logic while keeping the new byte-by-byte fallback. Alignment is attempted first, fallback is used only when alignment is invalid. `expected_size` is now computed correctly for each path, fixing the test failures caused by mixing aligned offsets with byte-step semantics in the previous commit.
Contributor
Author
|
@SweetVishnya hopefully fixed in 8bc16ae. Tested on my end and now the existing tests pass, and I am able to locate 2-byte gadgets with py ROPgadget.py --binary test-suite-binaries/elf-x64-bash-v4.1.5.1 --only "pop|ret" --align 2 --all | grep rdi |
SweetVishnya
approved these changes
Nov 22, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch fixes an issue where the
--alignoption prevents discovery of valid x86/x64 gadgets such aspop rdi; ret.ROPgadget currently uses the alignment value both as:
This coupling works for fixed-width ISAs (e.g. ARM Thumb, RISC-V compressed) but breaks on x86, where instruction lengths are variable and gadget starts often occur at ref - 1. With
--align 2, these byte offsets are never examined, causing valid x86 gadgets to be missed.Proposed fix:
This preserves the intended behavior for architectures where alignment is meaningful while restoring correct gadget enumeration on x86/x64.
Please note
I am currently unable to test this change on non-x86 architectures, so additional testing would be appreciated to ensure it does not introduce regressions on ARM, Thumb, or RISC-V. I can also provide a test ELF showcasing the issue if required.