pwndbg
pwndbg copied to clipboard
vis_heap_chunks: paginate results
Description
After heap chunk size if corrupted to a large size vis_heap_chunks
will print a ton of output. Paginating the results will help to bring back usefulness of the command in such situation.
Steps to reproduce
- Corrupt heap chunk with big chunk size
- Run
vis_heap_chunks
@arcz any chance you can provide a minimal PoC, e.g. one written in C so we can make this A) easier to develop, B) actually a test case in our test suite? :D
I have multiple PoCs for this, since it pretty much happens with every heap pwnable :) It's easy to reproduce, but I'm not sure exactly how to fix this. Should we use GDB's own pagination or implement something ourselves?
It's be nice to fix this in a way where we can make any command paginate in the future, in case they have similar issues. Or allow the user to customize which commands get paginated (sometimes I think it would be good to paginate heap
).
There are two options:
- use GDBs pagination
- use GDB command's repeat feature, when you hit
to repeat the command and get its output for e.g. further addresses. We have it implemented in our ArgparsedCommand
too and it works forhexdump
,telescope
and few more.
I think we could just go with 2)? I think that 1) does not stop processing code and only prevents from printing stuff?
Alternatively, we could set pagination only for this command to prevent way too long output and have it configurable.
I'm fine with option 2, but there's a few things to note:
- This might be a little more tricky than the other implementations. Generally, we always want to stop printing at a chunk boundary unless we hit some massive chunk, and at that point would we switch to printing a fixed set of address, or one chunk at a time, or something else? This won't be as simple as it is for
telescope
. - I didn't know about this repeat feature. We need to work on the discoverability of that somehow, and I'm not sure if tips are enough.
- It would be good to implement this in a more generic way, where I could make any command into a repeatable command with just a decorator or something like that. I see for
telescope
andhexdump
, we're essentially copying and pasting the same code with different variable names. I'm happy to help with this if we go this route.
Given all of that, it may be best to go with option 1. Ideally we could do something like set paginated-commands 'vis_heap_chunks heap'
, so the user could configure pagination to be enabled for any command, and I don't think this would be too hard to implement.
I want to make a feature for vis_heap_chunks
command.
We can set a default max chunk size to show with vis_heap_chunks
command.
If a chunk larger than the max chunk size for vis
, skip the middle part ( don't show it by default ).
We can use vis -a
to disable it temporary, and shows all the chunk content.
@disconnect3d what do you think about my comments above about option 1 and 2, and @lonnywong's suggestion to display all the chunks but cut out the middle of each chunk if the chunk is greater than some config option?
I think the pagination solutions more generally solve the problem than just removing the middle of the chunks. It works fine when you only have small chunks, but what if you have one big chunk that you want to see so you use vis -a
, but because the heap is corrupted there's some giant chunks after this target chunk, and now we have the same problem of having to scroll up to get to the information we want.
About a corrupted chunk, we shouldn't show the content after the top chunk. It may reduce the size in most cases.
@gsingh93
what do you think about my comments above about option 1 and 2
Regarding option 1 - which is using GDB pagination - I think we should drop this idea since its probably too hard to implement and not worth it (?).
Regarding option 2 - which is repeat functionality discoverability - well, that's a native GDB feature, so lots of GDB command have this, but not lots of Pwndbg commands. I guess we can add a tip + document it in docs in the future, but idk if we can do more about it. Perhaps we could show something like "<Hit enter for further results>" in commands implementing those when executing one of them for the very first time?
It would be good to implement this in a more generic way, where I could make any command into a repeatable command with just a decorator or something like that. I see for telescope and hexdump, we're essentially copying and pasting the same code with different variable names. I'm happy to help with this if we go this route.
That sounds good as long as it is really possible to have such abstraction for many commands. I mean, there is always some data you need to persist between repeated calls - usually a tracked address + maybe some flags.
@gsingh93 @lonnywong
Regarding what we should do with pagination here: I guess a good question we should ask ourselves here should be: what are the users using vis_heap_chunk
for? E.g. is it to see the ALL data of chunks in memory? Or maybe mainly their boundaries, for example to see if you overwritten a chunk or if a fake chunk looks fine?
I must admit I am not a heap expert and while I sometimes solve such challenges, you folks may have more experience with it than me, so I'd appreciate some feedback on how you use this.
On the other hand @lonnywong idea's to cut the middle part for really big chunks sounds better for me, as pagination for multiple biggest possible chunks may just be very annoying, when you will have to hit enter 5 times for example to get to the result of your interest?
vis
is my favorite command, and bins
is my second favorite.
I often use vis
to view the heap layout, it's better than heap
command.
vis
shows prev_size
, chunk_size
, fd
, bk
, fd_nextsize
, bk_nextsize
, and the freed chunk location etc. That's all I need in most cases.
Sometimes, I want to check a fake chunk is made as expected or not, most is inside a chunk smaller than 0x1000.
If there is a super large chunk, most cases are overflow with a lot A
( the chunk size is 0x4141414141414141 ).
About a corrupted chunk, we shouldn't show the content after the top chunk. It may reduce the size in most cases.
I had assumed that this issue was occurring because we weren't able to figure out where the top chunk was due to some corruption, but if that's not the case this could probably work.
Regarding option 1 - which is using GDB pagination - I think we should drop this idea since its probably too hard to implement and not worth it (?).
Can't we just enable GDB pagination before the command and disable it afterwards?
I guess we can add a tip + document it in docs in the future, but idk if we can do more about it.
Yea, help telescope
for example says "Recursively dereferences pointers starting at the specified address ($sp by default)", which made me think it would always be $sp
.
I had assumed that this issue was occurring because we weren't able to figure out where the top chunk was due to some corruption, but if that's not the case this could probably work.
The libc main_arena
has the address of the top chunk. It won't be corrupted on the heap, unless hacking the libc main_arena
.
Btw, telescope
works great, but sometimes it doesn't. e.g.:
pwndbg> telescope 0x7fffffffe220
# The result is great.
pwndbg>
# Click enter, it shows more, great.
# ... some other commands
pwndbg> telescope 0x7fffffffe220
# I doesn't start from 0x7fffffffe220.
# Since the address is the same, it starts from the end of the previous telescope.
I want to make an option to disable it. If the option is on, and it's not just an enter
, starts from the beginning.
@disconnect3d What do you think? about the telescope
.
About a corrupted chunk, we shouldn't show the content after the top chunk. It may reduce the size in most cases.
I disagree with this; if you've corrupted a chunk size field you probably want vis_heap_chunks
to honor that size field when inspecting the heap, rather than stopping arbitrarily at the top chunk.
I'm also not keen on snipping the contents of larger chunks, the point of vis_heap_chunks
is to show all chunk data, pwndbg has a heap
command if you just want to view chunk metadata.
I do agree that some way of limiting the command's output length could be a useful feature though.
I disagree with this; if you've corrupted a chunk size field you probably want vis_heap_chunks to honor that size field when inspecting the heap, rather than stopping arbitrarily at the top chunk.
Stop at the top chunk maybe a bad idea. There maybe a fake chunk larger than the top chunk. We should honor that size. What if the corrupted chunk larger than the whole heap, where should we stop?
I'm also not keen on snipping the contents of larger chunks, the point of vis_heap_chunks is to show all chunk data, pwndbg has a heap command if you just want to view chunk metadata.
I agree that the point of vis
is to show the chunk data. Just limit the large chunk size, and the size is configed by the user. I don't want to change the default behavior. Just show a hint on the first large chunk. Let the user notice that he can set a limit.
I think pagination is annoying. I would rather scroll up.
What if the corrupted chunk larger than the whole heap, where should we stop?
Tough call, the command currently stops at the end of the heap mapping. Personally I think assuming the user knows what they're doing and wants all the output is sensible. If they don't want to print the large chunk they can use the command's first argument to limit the number of chunks printed.
I don't want to change the default behavior. Just show a hint on the first large chunk. Let the user notice that he can set a limit.
I like this too 👍
I made a solution https://github.com/pwndbg/pwndbg/pull/1275 .
@gsingh93 @CptGibbon @lonnywong Do you folks think it still makes sense to paginate vis_heap_chunks
results now that we have max size in #1275?
We also optimized vis_heap_chunks
recently in https://github.com/pwndbg/pwndbg/pull/1678 and will probably optimize it further (I believe we can improve it by 10-20% more) so the long time to print all this could be less of a problem but ofc the many lines being printed could still be an issue.
I'd personally be up for closing this for now.
@disconnect3d It's all ok for me.
I'm okay with closing this, I like the solution by @lonnywong 👍