8000
Skip to content

Fix chunked reading of ELF files#865

Merged
zachriggle merged 3 commits intoGallopsled:devfrom
zachriggle:issue_864
Jan 24, 2017
Merged

Fix chunked reading of ELF files#865
zachriggle merged 3 commits intoGallopsled:devfrom
zachriggle:issue_864

Conversation

@zachriggle
Copy link
Copy Markdown
Member
@zachriggle zachriggle commented Jan 22, 2017

This introduces a new dependency on 'intervaltree', which is used
to automatically select segments which are relevant for a given
virtual-address range.

This also introduces a new ELF.memory attribute, which is the tree
of memory ranges, which point at the segment objects that describe
the memory range.

These are recalculated when ELF.address is adjusted, and we have
tests for that.

This also optimizes some of the other routines to avoid the wrapped
file-seeking, and to just use the mmap.

Fixes #864

@zachriggle zachriggle self-assigned this Jan 23, 2017
@zachriggle zachriggle added the bug label Jan 23, 2017
@zachriggle zachriggle added this to the Someday milestone Jan 23, 2017
@zachriggle
Copy link
Copy Markdown
Member Author

This is technically a bug that has existed since the ELF module was introduced in 8252b6c.

It hasn't ever broken anything before, and I'm hesitant about back-porting this medium-sized change.

I'm in the process of writing more tests, but it requires additional changes to pwnlib.asm. It looks like I actually stumbled across this bug a long time ago, but worked around it by forcing the assembler to emit padding into the ELF file.

@zachriggle zachriggle force-pushed the issue_864 branch 11 times, most recently from 9ecd1a8 to 584495f Compare January 24, 2017 16:57
This introduces a new dependency on 'intervaltree', which is used
to automatically select segments which are relevant for a given
virtual-address range.

This also introduces a new ELF.memory attribute, which is the tree
of memory ranges, which point at the segment objects that describe
the memory range.

These are recalculated when ELF.address is adjusted, and we have
tests for that.

This also optimizes some of the other routines to avoid the wrapped
file-seeking, and to just use the mmap.
@zachriggle zachriggle force-pushed the issue_864 branch 3 times, most recently from 6f4fe5d to aa8f8ec Compare January 24, 2017 17:58
@zachriggle
Copy link
Copy Markdown
Member Author

Greatly reduced the size of the changes, and added some changes to travis/install.sh which use Ubuntu Zesty's version of binutils and qemu-static.

It turns out that Ubuntu's Zesty packages -- for these specific packages -- run just fine on Precise. There's no need to pull from the Pwntools PPA.

@zachriggle
Copy link
Copy Markdown
Member Author

Ping @idolf for review

@zachriggle
Copy link
Copy Markdown
Member Author
zachriggle commented Jan 24, 2017

Restarted a new copy of the Travis build after deleting the cache files for this PR, to ensure that there are no lingering "broken but it's in cache so it works" issues.

Copy link
Copy Markdown
Contributor
@TethysSvensson TethysSvensson left a comment

Choose a reason for hiding this comment

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

LGTM.

Is there an easy way to add a test the hits a few of the edge cases of this? (e.g. non-continuous segments, overlapping segments)

@zachriggle
Copy link
Copy Markdown
Member Author

If you look at the changes, you should see there are tests for one of those scenarios (read spans non-contiguous segments). I can't get objdump to emit a malformed ELF which has overlapping segments.

What we could do is just have each DT_LOAD take precedence over any previous DT_LOAD regions, which would have a similar effect, and is presumably the "most correct" way to do it.

@zachriggle
Copy link
Copy Markdown
Member Author

Added bad24ee to address the second half of #865 (review)

@zachriggle zachriggle merged commit 2a59f5c into Gallopsled:dev Jan 24, 2017
@zachriggle zachriggle deleted the issue_864 branch January 24, 2017 18:41
@TethysSvensson TethysSvensson modified the milestones: 3.5.0, Someday Feb 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0