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

gef-remote broken #685

Closed
4 tasks done
theguy147 opened this issue Jul 28, 2021 · 8 comments
Closed
4 tasks done

gef-remote broken #685

theguy147 opened this issue Jul 28, 2021 · 8 comments
Labels

Comments

@theguy147
Copy link
Collaborator

  • Did you use the latest version of GEF from dev branch?
  • Is your bug specific to GEF (not GDB)? - Try to reproduce it running gdb -nx
  • Did you read the documentation first?
  • Did you check issues (including
    the closed ones) - and the PR?

Environment

  • OS: Ubuntu 20.04 focal
  • Kernel: x86_64 Linux 5.4.0-80-generic
  • GEF version:
GEF: rev:48a9fd74dd39db524fb395e7db528f85cc49d081 (Git - clean)
SHA1(/vagrant/geforig/gef.py): 848cdc87ba7c3e99e8129ad820c9fcc0973b1e99
GDB: 9.2
GDB-Python: 3.8

Problem

gef-remote command is broken. Instead of downloading the executable it downloads a invalid memory map of the binary as ASCII text. Furthermore that breaks all kind of other commands in a remote debugging session up to the point where debugging is not possible.

Steps to reproduce and minimalist test case

// compile with gcc -otest test.c
#include<stdio.h>
int main() {
	puts("Hello World!\n");
	return 0;
}
  1. gdbserver localhost:1337 ./test
  2. In second terminal: gdb -q -ex "gef-remote localhost:1337"
  3. Remeber the PID shown
  4. file /tmp/gef/<PID>/</path/to/test>

Example Output:

/tmp/gef/189674/vagrant/gef/test: ASCII text
00000000-ffffffff rwxp 00000000 00:00 0                    /tmp/gef/189674/vagrant/gef/test

Observed Results

Sometimes you can still step through the binary and sometimes everything crashes and gdb/GEF has to be killed.

Expected results

Some nice debugging sessions

Reason

Yet another fault caused by the new argparsing. git bisect reveals that the error was introduced in commit 782dd88 .

It appears better tests are also needed here to prevent such a regression next time around :)

@theguy147
Copy link
Collaborator Author

theguy147 commented Jul 28, 2021

ok, this is the issue (or at least one of them):

  • first gdb actually does download the correct executable from the remote target when executing target remote
  • then setup_remote_environment(...) gets called with pid = 0
  • gdb cant find that path on the remote host but does not throw an exception, instead it returns faulty data
  • the executable (and the other proc-files) get overwritten with that non-sensical data

one detail I don't quite understand in the code is why the executable gets downloaded twice anyway (gdb automatically does that when executing target remote and then setup_remote_envionment(...) does it again.

EDIT:
The pid is 0 because thats what gdb.selected_inferior().pid in get_pid() returns now and that is because at

gef/gef.py

Line 6168 in 48a9fd7

pid = args.pid if args.is_extended_remote and args.pid else get_pid()
the executable is simply not being debugged by gdb yet and thereby also no inferior to gdb.

@theguy147
Copy link
Collaborator Author

the solution appears to be quite simple: move that line down to after self.connect_target(...) and before ok("Attaching to {:d}".format(pid))

@theguy147
Copy link
Collaborator Author

theguy147 commented Jul 28, 2021

I created a PR for it but it is still missing better tests. I have to come up with something but I need to do some other things first.

EDIT:
I just realized that it doesn't need new tests for this exact issue. Instead get_pid() should raise an Error if pid == 0 so the existing tests can catch it.

EDIT:
actually I can't find any tests for gef-remote at all. So its back to writing some 😆

@theguy147
Copy link
Collaborator Author

I am not sure how to go about testing gef-remote. One problem is that it would add gdbserver as a dependency and the other is that for clean testing multi-threading would have to be introduced which seems like an overkill. Instead of the multi-threading it could just be done quick and dirty in a subprocess but I guess then we still need gdbserver for testing. What do you think?

@Grazfather
Copy link
Collaborator

We have a docker container for a reason. It can be annoying managing subprocesses like this, but gef remote has regressed before. I think it's worth adding tests.

I've seen some nice integration tests that actually manage individual containers. We might not need that, but I think subprocess calls to a remote is a reasonable start, especially if we don't need to worry about getting anything back from the gdbserver call.

@theguy147
Copy link
Collaborator Author

theguy147 commented Aug 1, 2021

so something like this?

def test_cmd_gef_remote(self):
    def start_gdbserver(exe, port=1234):
        return subprocess.Popen(["gdbserver", ":{}".format(port), exe], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
        
    def stop_gdbserver(gdbserver):
        """ stops the gdbserver and waits until it is terminated if it was still running. Needed to make the used port available again """
        if gdbserver.poll() is None:
            gdbserver.kill()
            gdbserver.wait()
        return
        
    # For each test something like:
    gdbserver = start_gdbserver("/some/path")
    # ... e.g. res = gdb_start_silent_cmd("vmmap", before=["gef-remote :1234]"])
    #            self.assertNotException(res)
    stop_gdbserver(gdbserver))

EDIT: this implies that gdbserver is in the PATH of the testing machine. If not then the test fails

@theguy147
Copy link
Collaborator Author

@Grazfather what do you think of my suggested way of testing?

@Grazfather
Copy link
Collaborator

Yep! That works, at least as a stop gap.

In the future we can mount the docker sock into the container of the test runner so that that container itself can start and stop containers as needed.

If gdbserver isn't already in the container we can just add it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants