martinradev / gdb-pt-dump

MIT License
138 stars 26 forks source link

add writeable and executable attribute for pwndbg #12

Closed lonnywong closed 2 years ago

lonnywong commented 2 years ago

pwndbg using page.x and page.w:

https://github.com/pwndbg/pwndbg/blob/9a141bccbb06599189b65b08e1bf53793ed44561/pwndbg/vmmap.py#L248

if page.w: flags |= 2
if page.x: flags |= 1
disconnect3d commented 2 years ago

@martinradev ping :). OTOH, couldn't we just use those functions directly in Pwndbg and add this field in there?

lonnywong commented 2 years ago

@martinradev ping :). OTOH, couldn't we just use those functions directly in Pwndbg and add this field in there?

@martinradev @disconnect3d Should we copy the source code to pwndbg? It will be easier to contribute.

martinradev commented 2 years ago

Should we copy the source code to pwndbg?

No, this would be a bad idea because both projects would go out-of-sync and things would break. I'm currently writing tests 1 for gdb-pt-dump and I would expect I make bug fixes.

If the integration is shown in 2, I don't think it's a reasonable way to integrate gdb-pt-dump. I may decide to refactor the project or completely break the gdb commands. A proper way is to contribute an API for gdb-pt-dump which could be used by pwndbg. Essentially, this would mean that pt would be the frontend and be using the gdb-pt-dump API to query address space ranges, etc.

I think self.w and self.x don't mean much in aarch64 world since it hides a lot of information.

lonnywong commented 2 years ago

I think self.w and self.x don't mean much in aarch64 world since it hides a lot of information.

The self.w and self.x is required for pwndbg.

Check https://github.com/pwndbg/pwndbg/blob/f9bd6352bbbbcc144d43cfe2e60459a1c684098e/pwndbg/vmmap.py#L246

        if page.w: flags |= 2
        if page.x: flags |= 1
lonnywong commented 2 years ago

Could you instead do?

def pwndbg_is_writeable(self):
        return is_user_writeable(self) or is_kernel_writeable(self)

def pwndbg_is_executable(self):
        return is_user_executable(self) or is_kernel_executable(self)

Simiar for pt_common.py and pt_riscv64_parse.py

WIth this change, it's very visible that this would only be used for pwndbg integration.

done