Don't skip over symbol at start of file in _populate_symbols #2451
Conversation
ThijsRay
force-pushed
the
fix_missing_start_symbol
branch
from
675d221 to
9e00ad7
Compare
Lets say we have an ELF with the following symbols
```
Symbol table '.symtab' contains 5 entries:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000000035 0 NOTYPE LOCAL DEFAULT 2 aaaa
2: 0000000000000022 0 NOTYPE LOCAL DEFAULT 2 bbbb
3: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 2 cccc
4: 0000000000000054 0 NOTYPE GLOBAL DEFAULT 2 dddd
```
Then pwnlib's ELF(binary).symbols will be `{'aaaa': 53, 'bbbb': 34, 'dddd': 84}`.
This is missing the symbol `cccc`, because its value is 0.
This change checks the name instead of the value, because the value can
be 0 if the symbol points to the beginning.
The new and correct value of pwnlib's ELF(binary).symbols will be
`{'aaaa': 53, 'cccc': 0, 'bbbb': 34, 'dddd': 84}`.
ThijsRay
force-pushed
the
fix_missing_start_symbol
branch
from
9e00ad7 to
0829371
Compare
|
Thank you for your contribution! Some tests are failing now so this doesn't seem to be enough. Can you investigate please? |
|
Actually, there appears to be a different issue causing tests to fail. |
|
I think the remaining failing tests are actually caused by this PR. e.g. the address 0 now is assigned some symbol |
|
It seems like my fix does not actually fix the right thing then. It is tricky because a value of 0 is used in both cases where the symbol starts at address 0, or that the symbol does not have a value. I will try to find a correct fix for this, but that might take some time as I am not very familiar with the code base of pwntools. |
|
Maybe we can restrict to some other attribute of Elf_Sym. The ELF structure you access in that code comes from pyelftools. The |
Lets say we have an ELF with the following symbols
Then pwnlib's ELF(binary).symbols will be
{'aaaa': 53, 'bbbb': 34, 'dddd': 84}. This is missing the symbolcccc, because its value is 0.This change checks the name instead of the value, because the value can be 0 if the symbol points to the beginning.
The new and correct value of pwnlib's ELF(binary).symbols will be
{'aaaa': 53, 'cccc': 0, 'bbbb': 34, 'dddd': 84}.Pwntools Pull Request
Thanks for contributing to Pwntools! Take a moment to look at
CONTRIBUTING.mdto make sure you're familiar with Pwntools development.Please provide a high-level explanation of what this pull request is for.
Testing
Pull Requests that introduce new code should try to add doctests for that code. See
TESTING.mdfor more information.Target Branch
Depending on what the PR is for, it needs to target a different branch.
You can always change the branch after you create the PR if it's against the wrong branch.
devdevstablestablebranchbetabetabranch, but notstabledevChangelog
After creating your Pull Request, please add and push a commit that updates the changelog for the appropriate branch.
You can look at the existing changelog for examples of how to do this.