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

Improve format string generator #1216

Merged
merged 21 commits into from Sep 24, 2019

Conversation

@bennofs
Copy link
Contributor

commented Oct 25, 2018

This PR improves the format string generator:

  • it now should build close to optimal length format string (for example, it will make use of long writes to write long sequences of zeros)
  • it works with null bytes in addresses (common on 64 bit) since it writes pointers after instructions (so no null
  • it has the ability to generate pointers/instructions separately (see fmt_str_split)
  • the interface should be such that it is relatively easy to manually craft all kinds of writes (using the AtomWrite primitive)
Copy link
Contributor

left a comment

Really cool stuff! It's missing some function-level documentation and doctests, and there are some general design questions (nested functions vs. an object with methods).

If we can get this cleaned up, it would be cool to merge.

pwnlib/fmtstr.py Outdated Show resolved Hide resolved
pwnlib/fmtstr.py Show resolved Hide resolved
pwnlib/fmtstr.py Outdated Show resolved Hide resolved
pwnlib/fmtstr.py Outdated Show resolved Hide resolved
pwnlib/fmtstr.py Outdated Show resolved Hide resolved
Copy link
Collaborator

left a comment

A huge +1, this module was pretty useless on 64-bit stuff so far. But there are some minor issues concerning python 3 compatibility or coding style.

pwnlib/fmtstr.py Outdated Show resolved Hide resolved
pwnlib/fmtstr.py Outdated Show resolved Hide resolved
pwnlib/fmtstr.py Outdated Show resolved Hide resolved
pwnlib/fmtstr.py Show resolved Hide resolved
@bennofs bennofs force-pushed the bennofs:improve-fmt branch from c771e76 to a02c035 Nov 19, 2018
@bennofs

This comment has been minimized.

Copy link
Contributor Author

commented Nov 19, 2018

I've done most of the cleanup now (there's still one nested function and some docs remaining).

Remaining todo before this is merge ready:

  • test on big-endian
  • more docs
  • remove one nested function

Optional:

  • end-to-end tests for all platforms (a binary with printf compiled for each platform -> verify write works)
@bennofs

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2018

Would it be ok to commit compiled, static binaries for tests to the repo? The binary for mips for example is about 600kb. Requiring a cross-compiler for tests is painful.

@zachriggle

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2018

@bennofs Yes, I believe this was part of my format string branch.

I would greatly prefer a flexible binary that allows lots of tests. You can see what I had in mind here: https://github.com/zachriggle/pwntools/tree/format-string/pwnlib/data/formatstring

If you're interested, you can also see how I planned to perform "automatic" format string exploration and exploitation here: https://github.com/zachriggle/pwntools/blob/format-string/pwnlib/formatstring.py

And look at the documentation here: https://github.com/zachriggle/pwntools/blob/format-string/docs/source/fmtstr.rst

Specifically, I wanted to leverage a prefix stub ("get me to the format string") and then use the Corefile tools to extract state.

@zachriggle

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2018

Ah, there is also an LD_PRELOAD tool in that folder that I was planning to use. Basically it replaces all of the PLT entries for *printf with a trap instruction (int3 or equivalent) so that a corefile is generated when the function gets called.

The intent here is to capture the register state and stack at the time *printf is invoked, with a cyclic() pattern on the stack (or in whatever buffer(s) we control). We can then automate everything else.

The obvious limitation is that this will trap on the first *printf call, but it was all just work-in-progress stuff.

@bennofs

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2018

I now verified that the code works correctly in a basic test for: MIPS, MIPS64, MIPSEL, MIPS64EL, X86, X86_64, PPC, PPC64, ARM, AARCH64. There is a Makefile that uses the cross compilers available on Ubuntu bionic (that's the version I used for tests) to build example vulnerable binaries for each arch.

@bennofs bennofs force-pushed the bennofs:improve-fmt branch from d8aad99 to 8a71847 Dec 14, 2018
@bennofs

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2018

Should be ready for merge now from my side.

pwnlib/fmtstr.py Outdated Show resolved Hide resolved
@Arusekk

This comment has been minimized.

Copy link
Collaborator

commented Jan 30, 2019

It looks like we must resolve some build problems before merging this, but looks ready to me

@zachriggle

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2019

Yes, I need to do a stable => beta => dev merge.

@zachriggle

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2019

For the sake of being able to run the tests provided, would you mind adding the printf binaries that you compiled? (And removing the .gitiginore)

@bennofs

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2019

I added the binaries. Wasn't sure on whether to add them since they increase the size of the git repo. The command to build them, run in ubuntu:18.04: make install-apt && make

@bennofs bennofs closed this Feb 1, 2019
@bennofs bennofs reopened this Feb 1, 2019
@zachriggle

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2019

They do inflate the size of the repo slightly, but having binaries that don't change due to toolchain / CI / etc. differences is preferable.

@Arusekk
Arusekk approved these changes May 8, 2019
@Arusekk Arusekk referenced this pull request Sep 24, 2019
@Arusekk Arusekk merged commit d2b9e48 into Gallopsled:dev Sep 24, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.5%) to 59.686%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.