Skip to content

Implemented the dump() feature in the DynELF. #1104

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

Merged
merged 2 commits into from
Feb 6, 2018
Merged

Implemented the dump() feature in the DynELF. #1104

merged 2 commits into from
Feb 6, 2018

Conversation

eyalitki
Copy link
Contributor

@eyalitki eyalitki commented Feb 6, 2018

Added a new feature to the DynELF class, the ability to dump the main memory pages of the leaked ELF: .text, .rodata, .data, ...

Here is the function's documentation:

   dump(libs = False)

   Dumps the ELF's memory pages to allow further analysis.

   Arguments:
        libs(bool, optional): True if should dump the libraries too (False by default)

   Returns:
       a list of tuples of the form:
       [(
           <virtual address base:     0x804a000>,
           <ELF's virtual address:    0x804af08>,
           <ELF's memory size:            0x144>,
           <binary memory content: '\x7fELF...'>,
        ),
       ]

This feature allows researchers to easily extract a remote "blackbox" ELF once an information leak vulnerability was found, so they could RE the program.

@zachriggle
Copy link
Member

zachriggle commented Feb 6, 2018

This at least needs an optimization to not dump read-only pages by default.

I'd also prefer that the returned data just be a dictionary of {address: bytes}. There's no need to specify the VM size since that's implicit in the data returned, and there's no need to specify the ELF's base address, just the vaddr of the pages.

@eyalitki
Copy link
Contributor Author

eyalitki commented Feb 6, 2018

Regardng the RO pages, this code is intended for black box CTFs, meaning that we do want to dump the text pages which are RO.

I accept the comment regarding the returned values, I can update it.

@zachriggle
Copy link
Member

I agree that the ability to dump RO pages is useful, just not sure it should be the default.

@zachriggle zachriggle self-assigned this Feb 6, 2018
@eyalitki
Copy link
Contributor Author

eyalitki commented Feb 6, 2018

OK. I will update the code.

Should I submit a new PR once the code will be updated?

@zachriggle
Copy link
Member

zachriggle commented Feb 6, 2018

Nope, if you just push another commit it'll automagically show up here! ❤️

Here's more info: https://stackoverflow.com/questions/9790448/how-to-update-a-pull-request-from-forked-repo

@zachriggle zachriggle merged commit 90f23df into Gallopsled:dev Feb 6, 2018
@eyalitki
Copy link
Contributor Author

eyalitki commented Feb 6, 2018

Happy to contribute to the project, thanks.

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.

2 participants