-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Allow libcdb search offline database #2259
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool, thank you! This is very useful to have for the libcdb module. I added my thoughts about the implementation inline.
I continued to optimize libcdb, mainly involving simplifying redundant code, enabling cache usage in offline mode, adding the Regarding the Now you have several options to configure the local libc-database: export PWNLIB_LOCAL_LIBCDB="/path/to/libc-database" context.local_libcdb="/path/to/libc-database" Or via
|
Cool, thank you for working on this. I'll look at your changes in more detail tomorrow. A f-string snuck in which fails the python 2 test, can you have a look at the CI results please? Adding a
|
Hi, have you finished code review? I've noticed that making libcdb default to using an offline database might be a better approach. If someone wants to use the online database, they can pass Currently, libcdb can automatically switch to offline mode if an offline database path is configured. To implement this functionality, I had to make some trade-offs in code elegance, and I've also found that this approach might not provide clear prompts to the users. Without any configuration, libcdb will default to online mode, and only by check the source code or documentation can users become aware of the presence of the offline mode. Nonetheless, this approach does offer a level of convenience. Or search for offline databases within the online mode. #2259 (comment) |
Alright, so here are the things needed in order to get this merged (opinionated, but for a reason):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay again, let's imagine it's tomorrow now :P
Currently, both offline and online search modes are enabled by default in libcdb. You can control them using the "offline" and "online" parameters. For instance, you can disable online search mode with the following code: libcdb.search_by_symbol_offsets({'puts': puts_leak, 'printf': printf_leak}, online=False) Additionally, I'm in the process of adding the functionality to download libc-database for libcdb's command-line application, as shown below: libcdb download all --save-path="/path/to/libc-database" How can I modify the |
I think you should not modify pwn.conf at all, but rather use a default path that is sensible ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is a rather big one, so I think it deserves splitting into two parts, because it looks very useful and I would like to merge it soon.
The first part should just introduce an offline provider, which is just another provider, always enabled, cannot be disabled, has a sensible default path, but can be configured by setting the local_libcdb context param (preferably via pwn.conf).
The second part can introduce a new optional offline mode, disabled by default, which would disable libc.rip/libcdb lookups.
pwnlib/libcdb.py
Outdated
""" | ||
|
||
with open(path, "rb") as fd: | ||
section = ELFFile(fd).get_section_by_name('.note.gnu.build-id') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it so much faster than our derived ELF()? I could bet it already has that functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm testing on the following code
from elftools.elf.elffile import ELFFile
def _get_elf_buildid(path):
""" Some ELF files lack the `.note.gnu.build-id` section. """
build_id = None
with open(path, "rb") as fd:
section = ELFFile(fd).get_section_by_name('.note.gnu.build-id')
if section:
build_id = section.data()
if not build_id:
log.debug("%s does not have a buildid", path)
return None
return binascii.b2a_hex(build_id).decode()
def _get_elf_buildid(path):
""" Some ELF files lack the `.note.gnu.build-id` section. """
build_id = ELF(path, checksec=False).buildid
if not build_id:
log.debug("%s does not have a buildid", path)
return None
return binascii.b2a_hex(build_id).decode()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use context.quiet
to silence the warnings (and they should be addressed anyway I think?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to address the time consumption, Is there any method to reduce the runtime of ELF? Using ELFFile takes only about 1 second, while ELF takes around 17 seconds.
And can you take a look at this issue #2304?
Did you mean that libcdb needs to provide the following two modes?
|
okey, I will try to submit it in two new PRs |
Do you need to conduct a code review first? or should I directly submit a new PR. This RP should split to three part, maybe four part.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left some more comments on the current implementation. I'd be awesome if you could proceed with splitting this up as you suggested above!
@@ -89,15 +91,15 @@ | |||
hash_parser.add_argument( | |||
'--unstrip', | |||
action = 'store_true', | |||
default = True, | |||
default = False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attempting to unstrip the libc should be enabled by default, so this can be cleaned up the other way around to only keep the --no-unstrip
but default to True
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okey, but I believe it would be better to set the default value of --no-unstrip
to False
. This way, I can pass not args.no_unstrip
as the value for unstrip
.
In the following code, by default, unstrip
is True
. If we use the --no-unstrip
, the unstrip
parameter will become False
.
symbols = {pairs[i]:pairs[i+1] for i in range(0, len(pairs), 2)}
matched_libcs = libcdb.search_by_symbol_offsets(symbols, raw=True, unstrip=not args.no_unstrip, offline=args.offline)
for libc in matched_libcs:
print_libc(libc)
fetch_libc(args, libc)
>>> context.local_libcdb = "/path/to/libc-database" | ||
>>> filename = search_by_symbol_offsets({'puts': 0x420, 'printf': 0xc90}, select_index=1, offline=True) | ||
>>> ELF(filename) | ||
ELF('/path/to/libc-database/db/libc6_2.31-0ubuntu9.12_amd64.so') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doctests are actually run. We'd need to setup a fake local libc-database in CI for this to work. I don't want to actually download a whole database in CI.
Sorry, this is an unintended PR. I was merely testing how doctests are executed. |
You can always execute your tests offline selectively like the following: python -m sphinx -b doctest docs/source docs/build/doctest docs/source/libcdb.rst |
Allow searching of offline libc-database under limited or poor network conditions.
Add the offline parameter to libcdb.search_by_symbol_offsets, and searched according to the format of libc-database, which is the open source project used by http://libc.rip.
steps:
git clone https://github.com/niklasb/libc-database cd libc-database ./get ubuntu
export LOCAL_LIBC_DATABASE=/path/to/libc-database