Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix sorting in MachO::Binary::extend_section, rework extend_section tests #1152

Closed

Conversation

DzenIsRich
Copy link
Contributor

See individual commit messages.

romainthomas and others added 4 commits January 5, 2025 17:45
As demonstrated by @lebr0nli in the sample `ELF/issue_dynamic_table.elf`,
the PT_DYNAMIC segment can have an "invalid" offset while still running
correctly.
This commit changes the logic of the dynamic table processing by using
the PT_LOAD segment that wraps this table
If there are empty sections and non-empty section at the same offset, we
want to shift only empty sections.

The expected behavior is that we sort sections in ascending order, and
then `sections_to_shift` is trimmed, such that the last element becomes
`section`.

Previously, we were sorting in descending order, while keeping empty
sections in the same order. Due to this we were doing trimming
incorrectly.
The previous implementation of the test was expecting unsupported
behavior from LIEF.

LIEF currently does not support arbitrary calls to add_section and
extend_section and keeping the layout of the binary correct.
Currently it is responsibility of a user to ensure that the final layout
of the binary is well-formed.

This patch splits test into two cases:
1. Repeated calls to add_section followed by extend_section using
   different alignment values.
2. Multiple calls to add_section and then multiple calls to
   extend_section using the same alignment values.
@romainthomas
Copy link
Member

Thank you!

I manually merged the PR through 60de925 (because of a force push)

@DzenIsRich DzenIsRich deleted the zmpr-fix-macho-extend-section branch January 13, 2025 08:22
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants