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

Add functions for retrieving process mappings #2371

Open
wants to merge 54 commits into
base: dev
Choose a base branch
from

Conversation

k4lizen
Copy link

@k4lizen k4lizen commented Mar 21, 2024

Closes #2369

  • implement process.maps which is a wrapper around util.proc.memory_maps
    • calls on self.pid, parses string address field and permissions field
  • get_mapping function which finds mappings given a "path" to look
  • make stack_mapping, heap_mapping, vdso_mapping, vvar_mapping, elf_mapping which hook into get_mapping, and libc_mapping, musl_mapping
    • they return all the data that util.proc.memory_maps returns (so lots!)
  • address_mapping which allows the user to get a mapping for a supplied address
    • useful for checking where an address belongs like: print(p.address_mapping(is_this_stack_addr).path == '[stack]')
  • lib_size which returns the size of a loaded shared library given the path

Also Closes #2370
I didn't really touch .libs() or .libc since they have a different return signature. Also .libs() has

        if not maps_raw:
            import pwnlib.elf.elf

            with context.quiet:
                return pwnlib.elf.elf.ELF(self.executable).maps

Which makes little sense to me, because if /proc/<pid>/maps really fails and it goes to this check, if ASLR is enabled .libs() will just quietly return wrong addresses instead of erroring out, which seems counterintuitive.
Fun fact: now you can do stuff like print(p.stack_mapping().perms.execute) !! (prints True/False)

@k4lizen k4lizen marked this pull request as draft March 21, 2024 07:06
@k4lizen
Copy link
Author

k4lizen commented Mar 21, 2024

Oh didn't know it had to support python2.7

@k4lizen k4lizen marked this pull request as ready for review March 21, 2024 07:43
@k4lizen
Copy link
Author

k4lizen commented Mar 21, 2024

@peace-maker

Next to the hardcoded properties, having a shortcut to receive the base address of any mapping without having to filter the .libs() result manually would be great too.

Not sure what exactly you meant by this. get_mapping_location probably does that? Though the path_value does need to be an exact match.

@peace-maker
Copy link
Member

Yes, I was imagining a parameter to .libs() to get the base address of the passed lib instead of a dict.

@k4lizen
Copy link
Author

k4lizen commented Mar 21, 2024

Sorry for late push, address_mapping is a good function for this PR.

Yes, I was imagining a parameter to .libs() to get the base address of the passed lib instead of a dict.

So do I leave it as is, change the functions I added, or add an overload to .libs() which takes in an additional argument and returns only the base address?
It is non-trivial for the user to e.g. find the size/end location of libc if we just return the base address, so that is something that would be nice to have in some form somewhere.

Copy link
Member

@peace-maker peace-maker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I've only some suggestions around the docs.

pwnlib/tubes/process.py Outdated Show resolved Hide resolved
pwnlib/tubes/process.py Outdated Show resolved Hide resolved
pwnlib/tubes/process.py Outdated Show resolved Hide resolved
@k4lizen
Copy link
Author

k4lizen commented Apr 22, 2024

A doctest failed but it isnt mine?

@k4lizen
Copy link
Author

k4lizen commented Apr 23, 2024

^ Other than that, I think thats it. (you can add the musl doctests after you do the musl-tools stuff I guess).

Btw, after thinking about it a bit (and while making the tests), I feel like the various something_location() functions are redundant and should probably be removed. That use-case was something I needed in my project but address_mapping now does what I need and the niche of "has multiple mappings in memory that are continous and cares about what their total size is" is too narrow, especially since just doing something like

libc_mappings = p.libc_mapping(single=False)
size = libc_mappings[-1].end - libc_mappings[0].start

is easy enough. Also calling vdso_location() instead of vdso_mapping() is simply slower and returns less information, since vdso will only ever be one mapping, so it might actually be harmful to leave it in. I'd still leave get_mapping_location in, though maybe rename it and make it return only the size.

What do you think?

@peace-maker
Copy link
Member

Yes, please remove redundant API. Maybe returning some class instead of a simple mapping list which has a function to give you the total mapping size? Don't know how usable that is

@k4lizen
Copy link
Author

k4lizen commented Apr 30, 2024

Okay so since vvar and vdso are only one mapping and stack and heap don't have contiguity guarantees it only makes sense from a user perspective to ask for the size of shared libraries (and the elf I guess?), so I replaced all the location stuff with lib_size(path) which does what it says (seemed simpler than a class approach).

@k4lizen
Copy link
Author

k4lizen commented Apr 30, 2024

If you agree with the changes, the PR is done / ready for (re)review.

@k4lizen
Copy link
Author

k4lizen commented Apr 30, 2024

Oh and as I said, it might be good to look into why https://github.com/Gallopsled/pwntools/actions/runs/8789031621/job/24117819520 failed.

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.

[dev] Bad libc mapping detection Get address of stack, heap, binary in memory of process
2 participants