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

helpers: Added dynamic symbol resolver #128

Merged
merged 5 commits into from Feb 16, 2022

Conversation

guyarb
Copy link
Contributor

@guyarb guyarb commented Feb 13, 2022

Dynamic symbols are very important to attach u(ret)probe on common libraries such as openssl, libc, etc.

issue: #129

Dynamic symbols are very important to attach u(ret)probe on common libraries such as openssl, libc, etc.
Copy link
Member

@grantseltzer grantseltzer left a comment

This is great, thank you! Just one comment, let me know what you think!

helpers/elf.go Outdated

// DynamicSymbolToOffset attempts to resolve a dynamic 'symbol' name in the binary found at
// 'path' to an offset. The offset can be used for attaching a u(ret)probe
func DynamicSymbolToOffset(path, symbol string) (uint32, error) {
Copy link
Member

@grantseltzer grantseltzer Feb 15, 2022

Choose a reason for hiding this comment

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

What do you think about adding this logic to SymbolToOffset instead? If the named symbol isn't in the symbol table, check dynamic symbols. Is it possible to have a name collision?

Copy link
Contributor Author

@guyarb guyarb Feb 15, 2022

Choose a reason for hiding this comment

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

I'm in favor of your suggestion, I didn't want to "break" anyone using the original function, so ill change the code accordingly.

regarding the name collision - so the short answer would be yes, but it is only because the dynamic symbol table contains exported functions that will appear in the regular symbol table.

a nice answer in stack overflow https://reverseengineering.stackexchange.com/a/21623

Copy link
Contributor Author

@guyarb guyarb Feb 15, 2022

Choose a reason for hiding this comment

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

created a simple SO (followed https://www.cprogramming.com/tutorial/shared-libraries-linux-gcc.html)
and checked both the regular symbol table and the dynamic symbol table
you can see the duplications
image

@guyarb
Copy link
Contributor Author

guyarb commented Feb 15, 2022

This is great, thank you! Just one comment, let me know what you think!

Thanks!
commented + fixed

Copy link
Member

@grantseltzer grantseltzer left a comment

Couple of small suggestions (let me know WYT) but otherwise LGTM!

helpers/elf.go Outdated Show resolved Hide resolved
helpers/elf.go Outdated Show resolved Hide resolved
guyarb and others added 2 commits Feb 16, 2022
Co-authored-by: grantseltzer <grantseltzer@gmail.com>
Co-authored-by: grantseltzer <grantseltzer@gmail.com>
@guyarb
Copy link
Contributor Author

guyarb commented Feb 16, 2022

Couple of small suggestions (let me know WYT) but otherwise LGTM!

Great suggestions!
committed them directly.

helpers/elf.go Outdated Show resolved Hide resolved
Copy link
Member

@grantseltzer grantseltzer left a comment

I messed up that error name woops lol, thanks for the contribution!

@grantseltzer grantseltzer merged commit d2980ae into aquasecurity:main Feb 16, 2022
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants