Open k4lizen opened 4 months ago
Oh didn't know it had to support python2.7
@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.
Yes, I was imagining a parameter to .libs() to get the base address of the passed lib instead of a dict.
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.
Done
I just noticed this doesn't have any tests. Can you add doctest examples to all functions you added please? I'll try to add them eventually if you don't have the time :)
I can add them.
Though I'm not sure exactly how I would test for musl. Any recommendations on that front?
The best way to do the doctest is probably by inspecting /proc/<pid>/maps
and comparing it with pwntools output (though I'm pretty sure the dependancy we pull in for the mappings is inspecting /maps itself xd). I'll do an IO operation before the check so the process has time to load libc, heap etc.
By the way I forgot to add the private
and shared
fields to the permissions
struct so I'll do that as well.
Maybe we can get away with stubbing out the expected output using ...
and ELLIPSIS instead of checking for the exact values. psutil parses /proc too indeed.
For musl we could install musl-tools
and build a simple test application in CI? We already have testing binaries in the pwnlib/data/elf
folder. Those should probably be rebuilt dynamically instead of having binaries in git but that's another problem.
Maybe we can get away with stubbing out the expected output using ... and ELLIPSIS instead of checking for the exact values.
I don't see how that would help with musl, like if I call p.musl_mapping()
it will just return None. I mean I can ... the whole output but that would IMO be worse then not having any tests for that.
For musl we could install musl-tools and build a simple test application in CI? We already have testing binaries in the pwnlib/data/elf folder.
Sounds good, though I don't really know CI so I can't help you there.
A doctest failed but it isnt mine?
^ 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?
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
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).
If you agree with the changes, the PR is done / ready for (re)review.
Oh and as I said, it might be good to look into why https://github.com/Gallopsled/pwntools/actions/runs/8789031621/job/24117819520 failed.
Closes #2369
process.maps
which is a wrapper aroundutil.proc.memory_maps
get_mapping
function which finds mappings given a "path" to lookstack_mapping
,heap_mapping
,vdso_mapping
,vvar_mapping
,elf_mapping
which hook intoget_mapping
, andlibc_mapping
,musl_mapping
util.proc.memory_maps
returns (so lots!)address_mapping
which allows the user to get a mapping for a supplied addressprint(p.address_mapping(is_this_stack_addr).path == '[stack]')
lib_size
which returns the size of a loaded shared library given the pathAlso Closes #2370 I didn't really touch
.libs()
or.libc
since they have a different return signature. Also.libs()
hasWhich 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 likeprint(p.stack_mapping().perms.execute)
!! (prints True/False)