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

Implement "got-audit" command. #112

Merged
merged 2 commits into from May 7, 2024
Merged

Implement "got-audit" command. #112

merged 2 commits into from May 7, 2024

Conversation

gordonmessmer
Copy link
Contributor

@gordonmessmer gordonmessmer commented Apr 28, 2024

Description

The "got-audit" command examines the symbols in the GOT, and prints a list of symbols and the path of the file that provides the mapped memory that the value points to. Additionally, it will print errors if a symbol is provided by multiple shared libraries, or if a symbol in the GOT points to a library that doesn't provide it.

To prevent future attacks similar to the liblzma attack, I'd like to implement a tool to audit the GOT of a running process which can be used in Fedora (and elsewhere) test infrastructure to look for signs of tampering.

How Has This Been Tested ?

"Tested" indicates that the PR works and the unit test (i.e. make test) run passes without issue.

  • x86-32
  • x86-64
  • ARM
  • AARCH64
  • MIPS
  • POWERPC
  • SPARC
  • RISC-V

Checklist

  • My code follows the code style of this project.
  • My change includes a change to the documentation, if required.
  • If my change adds new code,
    adequate tests have been added.
  • I have read and agree to the
    CONTRIBUTING document.

@gordonmessmer
Copy link
Contributor Author

This PR has been tested against gef with hugsy/gef#1097

I've tested it with interactive use and running the got_audit.py unit test.

@gordonmessmer gordonmessmer marked this pull request as ready for review April 29, 2024 01:05
@gordonmessmer
Copy link
Contributor Author

Before this is merged:

I notice that the symbols in the GOT correspond to those in the .rela.plt section of the ELF executable, but they don't include the PLT section of shared libraries.

That is, if I examine the GOT of a running /bin/login process, I see the symbols that the executable uses. But the executable is linked to libpam.so.0, and libpam.so.0 uses dlopen, and dlopen isn't present in the GOT.

Are there additional offset tables that need to be examined?

I can follow up on this later and improve the command when I figure it out, but if you happen to know that I've missed something, I'd appreciate any pointers.

@gordonmessmer
Copy link
Contributor Author

Checking in:

Is this PR waiting on any action on my part?

Copy link
Owner

@hugsy hugsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hugsy
Copy link
Owner

hugsy commented May 5, 2024

Hi @gordonmessmer ,

I got pretty busy lately with little time to review this PR

Checking in:

Is this PR waiting on any action on my part?

I got pretty busy lately with little time to spare on gef/gef-extras. Sorry for the delay, tests are running now. If they pass, I'll merge it

@gordonmessmer
Copy link
Contributor Author

Tests won't pass until after hugsy/gef#1097 is merged. :)

@hugsy
Copy link
Owner

hugsy commented May 5, 2024

Actually tests are failing for another reason (type hints)

@gordonmessmer
Copy link
Contributor Author

I see two test failures: FAILED tests/commands/capstone_disassemble.py::CapstoneDisassembleCommand::test_cmd_capstone_disassemble and FAILED tests/commands/got_audit.py::GotAuditCommand::test_cmd_got_audit

The first fails because gdb reports "cs" is an unknown command: https://github.com/hugsy/gef-extras/actions/runs/8869653413/job/24609506965?pr=112#step:9:129

...and the second fails because "nm" isn't found in gef.session.constants: https://github.com/hugsy/gef-extras/actions/runs/8869653413/job/24609506965?pr=112#step:9:273

Unless I'm misreading the test output... which is always possible.

I think if you run the tests again, the second test failure will go away. I don't think the first is related to the changes in this PR, so that one will probably fail again.

I'll look at the type hints again... I could swear those were all in place. 😕

@gordonmessmer
Copy link
Contributor Author

Type hints look like they're consistent with other modules. If I'm overlooking something, I'd be grateful for any pointers.

@gordonmessmer
Copy link
Contributor Author

The "cs" failure looks like the same thing that #110 is intended to fix.

@gordonmessmer
Copy link
Contributor Author

Thanks for running the tests. I see that the got-audit test fails in the container because the library name is different than on the platform where I ran unit tests. :(

I'll update the tests later today and push a change to address that.

@hugsy
Copy link
Owner

hugsy commented May 7, 2024

The "cs" failure looks like the same thing that #110 is intended to fix.

Correct, still a WIP so I don't mind it for your PR

I'll update the tests later today and push a change to address that.

Sounds good to me, I'll wait for it and merge your PR after that.

Thanks for your work!

Cheers

@hugsy hugsy added this to the 2024.05 milestone May 7, 2024
@hugsy hugsy merged commit 5d96909 into hugsy:main May 7, 2024
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants