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

Add cyclic default args to context #1049

Merged
merged 5 commits into from Oct 12, 2017

Conversation

Projects
None yet
2 participants
@zachriggle
Contributor

zachriggle commented Oct 11, 2017

Specifically, this adds context.cyclic_alphabet and context.cyclic_size

This helps cut down on writing n=8 all the time for users who use 8-byte
subsequences on 64-bit architectures.

Exposing the default alphabet means that users can avoid "bad bytes" in
cyclic patterns which are generated by pwntools internally (e.g. ROP padding)

Some pwnlib.util.packing docstrings were changed to clarify that word_size is
in bits.

The doctests in pwnlib.util.cyclic were updated and extended to have more
and better doctests, as well as better exaples.

@zachriggle zachriggle requested a review from Idolf Oct 11, 2017

@zachriggle zachriggle self-assigned this Oct 11, 2017

zachriggle added some commits Oct 11, 2017

Add cyclic default args to context
Specifically, this adds context.cyclic_alphabet and context.cyclic_size

This helps cut down on writing "n=8" all the time for users who use 8-byte
subsequences on 64-bit architectures.

Exposing the default alphabet means that users can avoid "bad bytes" in
cyclic patterns which are generated by pwntools internally (e.g. ROP padding)

Some pwnlib.util.packing docstrings were changed to clarify that word_size is
in bits.

The doctests in pwnlib.util.cyclic were updated and extended to have more
and better doctests, as well as better exaples.
@zachriggle

This comment has been minimized.

Show comment
Hide comment
@zachriggle

zachriggle Oct 11, 2017

Contributor

@Idolf A small change which I think may have been a bug is that the old code used pack(..., "all") when an integer value was provided. I think it should have used n, since otherwise if my alphabet included e.g. \x00 things would get weird. I added a doctest for this specific case, demonstrating that it works now.

The old behavior is pretty clearly wrong:

>>> cyclic_find(0, alphabet=unhex('DEADBEEF00'))
4
>>> cyclic(16, alphabet=unhex('DEADBEEF00'))
'\xde\xde\xde\xde\xad\xde\xde\xde\xbe\xde\xde\xde\xef\xde\xde\xde'
Contributor

zachriggle commented Oct 11, 2017

@Idolf A small change which I think may have been a bug is that the old code used pack(..., "all") when an integer value was provided. I think it should have used n, since otherwise if my alphabet included e.g. \x00 things would get weird. I added a doctest for this specific case, demonstrating that it works now.

The old behavior is pretty clearly wrong:

>>> cyclic_find(0, alphabet=unhex('DEADBEEF00'))
4
>>> cyclic(16, alphabet=unhex('DEADBEEF00'))
'\xde\xde\xde\xde\xad\xde\xde\xde\xbe\xde\xde\xde\xef\xde\xde\xde'

zachriggle added some commits Oct 11, 2017

Add an example showing an alphabet with NUL bytes
This is important because old code would use pack(..., "all") when an integer
value was provided, instead of using the correct width.

@zachriggle zachriggle merged commit 8f3839d into Gallopsled:dev Oct 12, 2017

3 checks passed

codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.05%) to 57.86%
Details
@Idolf

This comment has been minimized.

Show comment
Hide comment
@Idolf

Idolf Oct 13, 2017

Contributor

I don't really have the cycles to check if this is correct -- but you've added tests so I'm inclined to trust you. 👍

Contributor

Idolf commented Oct 13, 2017

I don't really have the cycles to check if this is correct -- but you've added tests so I'm inclined to trust you. 👍

@Idolf Idolf added this to the 3.11.0 milestone Oct 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment