From 790ba574c04cfbf5b12ac8f336cf4fffcdb98faf Mon Sep 17 00:00:00 2001 From: Disconnect3d Date: Tue, 14 Sep 2021 00:27:59 +0200 Subject: [PATCH] Refactor pwndbg.proc.exe and pwndbg.proc.get_file Revert the change from 3e4ad60 and make the `pwndbg.proc.get_file` to strip the "target:" prefix instead of the `pwndbg.proc.exe`. This way, we will prevent bugs when pwndbg code would use `pwndbg.proc.exe` on remote targets but not pass the returned path to `pwndbg.proc.get_file` to get the real remote file and instead use the local one (if it exists in the same path). Additionally, we assert the `path` passed to `pwndbg.proc.get_file` so we prevent incorrect use of the function when an absolute path or not a remote path is passed to it. --- pwndbg/file.py | 10 ++++++++++ pwndbg/proc.py | 20 ++++++++------------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pwndbg/file.py b/pwndbg/file.py index 97a7b2fb7..37313c3f6 100755 --- a/pwndbg/file.py +++ b/pwndbg/file.py @@ -22,13 +22,23 @@ def get_file(path): Downloads the specified file from the system where the current process is being debugged. + If the `path` is prefixed with "target:" the prefix is stripped + (to support remote target paths properly). + Returns: The local path to the file """ + assert path.startswith('/') or path.startswith('target:'), "get_file called with incorrect path" + + if path.startswith('target:'): + path = path[7:] # len('target:') == 7 + local_path = path qemu_root = pwndbg.qemu.root() + if qemu_root: return os.path.join(qemu_root, path) + elif pwndbg.remote.is_remote(): if not pwndbg.qemu.is_qemu(): local_path = tempfile.mktemp(dir=pwndbg.symbol.remote_files_dir) diff --git a/pwndbg/proc.py b/pwndbg/proc.py index 002438aa6..05dd5ca2d 100644 --- a/pwndbg/proc.py +++ b/pwndbg/proc.py @@ -59,21 +59,17 @@ class module(ModuleType): @property def exe(self): """ - We lstrip "target:" for remote debugging. Otherwise, the - `pwndbg.file.get_file(pwndbg.proc.exe)` would not work on remote targets. + Returns the debugged file name. - This should not be a problem on local targets as `gdb.current_progspace().filename` - seems to return absolute paths. + On remote targets, this may be prefixed with "target:" string. + See this by executing those in two terminals: + 1. gdbserver 127.0.0.1:1234 /bin/ls + 2. gdb -ex "target remote :1234" -ex "pi pwndbg.proc.exe" - DO NOT USE THIS if you want to process the debugged file with another tool. - For this, use `pwndbg.file.get_file` as it supports remote target. + If you need to process the debugged file use: + `pwndbg.file.get_file(pwndbg.proc.exe)` """ - fn = gdb.current_progspace().filename - - if fn.startswith('target:'): - return fn[7:] # len('target') == 7 - - return fn + return gdb.current_progspace().filename @property def mem_page(self):