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

Remove AddressFamily. prefix from address_family #1316

Merged
merged 2 commits into from
Jan 7, 2020

Conversation

FelixMartel
Copy link
Contributor

@FelixMartel FelixMartel commented Jun 20, 2019

Small bug where the address family ends up prefixed with AddressFamily.. Which won't assemble since the constant is not defined.

@zachriggle
Copy link
Member

Looks like this breaks some tests, specifically the cases where we pass an integer in as the AddressFamily. Can you add a unit test for the case that failed, which this change fixes?

Separately, @Arusekk it looks like the Python3 changes pass with this PR and the Python2 fails. Can you look into this and make sure we're running all fo the tests on Py3?

@zachriggle zachriggle requested review from zachriggle and Arusekk June 26, 2019 01:35
@Arusekk
Copy link
Member

Arusekk commented Jun 26, 2019

The correct way to deal with it is to check if addressfamily is an instance of int, and if so, cast it to int (because in py3 it is some enum subclassing int). I will do it soon.

@zachriggle
Copy link
Member

That will probably break some of the more fragile tests, but also makes the assembly less readable.

>>> print shellcraft.i386.linux.socket()
    /* open new socket */
    /* socketcall(AF_INET (2), SOCK_STREAM (1), 0) */
    /* push 0 */
    push 1
    dec byte ptr [esp] /* socklen_t addrlen */
    /* push SOCK_STREAM (1) */ <==========
    push 1     /* sockaddr *addr */
    /* push AF_INET (2) */
    push 2       /* sockfd */

The sockaddr routine currently returns a tuple which isn't ideal anyway, it should probably return a namedtuple at least, and we can have the AddressFamily object wrapped with something else or just add a .name (fourth value) or create a Python3-like AddressFamily class for Python2 which does have a .name property.

@Arusekk Arusekk changed the base branch from dev3 to dev December 27, 2019 18:28
@Arusekk Arusekk merged commit 6d340bf into Gallopsled:dev Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants