Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: exclude the latest capstone because it breaks ROPgadget #2492

Open
wants to merge 1 commit into
base: stable
Choose a base branch
from

Conversation

pmondon
Copy link

@pmondon pmondon commented Oct 22, 2024

FIX for this issue : #2491

The idea is that capstone push a new tag that is an Alpha and this breaks ROPgadget.
As ROPgadget does not have requirements to fix this issue (they only mention dependency on capstone) I thought that I could modify the requirements inside of pwntools so it excludes the breaking tag.

@pmondon
Copy link
Author

pmondon commented Oct 22, 2024

I don't think this is my fault is it ?
image

@pmondon
Copy link
Author

pmondon commented Oct 22, 2024

@peace-maker
Copy link
Member

https://github.com/capstone-engine/capstone/blob/next/docs/cs_v6_release_guide.md#notes-about-aarch64-and-systemz-renaming
There is no such compatibility hack in the python bindings (yet?), but I'd rather monkey patch this ourselves than block users from updating capstone in a pwntools env.

import capstone
if hasattr(capstone, 'CS_ARCH_AARCH64') and not hasattr(capstone, 'CS_ARCH_ARM64'):
  setattr(capstone, 'CS_ARCH_ARM64', capstone.CS_ARCH_AARCH64)
  setattr(capstone, '__all__', capstone.__all__ + ['CS_ARCH_ARM64'])
import ropgadget
# ...

The coveralls CI breakage is unrelated.

@pmondon
Copy link
Author

pmondon commented Oct 22, 2024

With the proposed fix there is no preventing the user from updating.
It is just that by default when installing pwntools the version picked from the capstone engine excludes the 6.0.0.a1 which is an alpha anyway. Is it normal that we could install an alpha by default or should we focus on stable versions of packages?

@peace-maker
Copy link
Member

We try to be as permissive as possible for package versions to make installing pwntools a no-brainer into any environment. The version restriction causes a conflict to be printed when trying out the new capstone and will result in the initial error in the ROP code too.

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
pwntools 4.13.1 requires capstone!=6.0.0a1,>=3.0.5rc2, but you have capstone 6.0.0a1 which is incompatible.

@pmondon
Copy link
Author

pmondon commented Oct 22, 2024

I understand the issue. Then I don't know how to fix this for now...

@peace-maker
Copy link
Member

This is only transitive though until ROPgadget is compatible with the capstone change. I've opened an issue for now JonathanSalwan/ROPgadget#202

@peace-maker
Copy link
Member

I understand the issue. Then I don't know how to fix this for now...

I think adding that monkey patching before importing ropgadget for the first time should work.

import ropgadget

It's weird that pip selects a pre-release version when resolving the dependencies. I would think this wasn't the case before. That version is out since 30.09. and this is the first time this pops up.

@pmondon
Copy link
Author

pmondon commented Oct 22, 2024

What I did is downgrade capstone locally.
Do you want to merge the monkey patch into stable or simply leave that as an issue until we hear more from ROPgadget?

@Arusekk
Copy link
Member

Arusekk commented Oct 22, 2024

Thanks for the fix, but I think pwntools is not the correct place to deal with this.
This is certainly not an issue with pwntools, as we state our deps correctly. While we do use ROPgadget in a strange way, this is a thing for them to exclude CS 6+ for now, and to later adapt to the API change, but it is CS' fault of doing a breaking change (they did use a semantic version bump though, so they should be excused). If you wish to fix this issue 'correctly' you should have filed the issue against ROPgadget (peace-maker has, thank you). In case ROPgadget refuses to fix it in the future, we can work around it. But for now the bug is not a serious issue for Pwntools.

@Arusekk Arusekk linked an issue Oct 22, 2024 that may be closed by this pull request
@peace-maker
Copy link
Member

It's weird that pip selects a pre-release version when resolving the dependencies. I would think this wasn't the case before. That version is out since 30.09. and this is the first time this pops up.

The problem is with our dependency mentioning a prerelease version capstone>=3.0.5rc2 which causes pip to look for newer prereleases too. So we could bump our minimum capstone version to something without a prerelease too.

I'd be fine to let this sit for a while and wait for a fix from ropgadget. If you stumble on this while trying to use the ROP class with an aarch64 binary, there are workarounds posted here.

@peace-maker peace-maker added the dependencies Pull requests that update a dependency file label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Capstone update breaks ropgadget
3 participants