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
debug/pe: fails to parse variable length optional header #32126
Comments
Change https://golang.org/cl/177959 mentions this issue: |
@prash2611 thank you for reporting the problem. I will review your change when I have time. Alex |
Thanks @alexbrainman, looking forward to your review comments. |
Hi. I tested this code. I tested using Ubuntu 18.04 kernel (vmlinuz-4.18.0-25-generic) taka@ubuntu:~/tmp$ sudo cp /boot/vmlinuz-4.18.0-25-generic .
taka@ubuntu:~/tmp$ sudo chmod +r vmlinuz-4.18.0-25-generic
taka@ubuntu:~/tmp$ go version
go version go1.12.7 linux/amd64
taka@ubuntu:~/tmp$ go run pe.go vmlinuz-4.18.0-25-generic
invalid format
exit status 1 With the patch, my program worked correctly. taka@ubuntu:~/tmp$ ~/git/go/bin/go version
go version devel +0cb90728c9 Mon Jun 17 11:31:57 2019 -0700 linux/amd64
taka@ubuntu:~/tmp$ ~/git/go/bin/go run pe.go vmlinuz-4.18.0-25-generic
Header64
dataDir:[{0 0} {0 0} {0 0} {0 0} {8550272 1912} {0 0} {0 0} {0 0} {0 0} {0 0} {0 0} {0 0} {0 0} {0 0} {0 0} {0 0}] |
Thanks @nokute78, glad to know that the patch solves your issue! I will let @alexbrainman comment on when it will be merged in master and GA. |
I am glad to know. Thank you for confirming. We cannot merge this change onto master branch until after go1.13 is released. Everyone is waiting for that. Alex |
@prash2611 @alexbrainman I’m also looking forward to Go 1.13. |
The debug/pe package assumes there are always 16 entries in DataDirectory in OptionalHeader32/64 ref pe.go: ... NumberOfRvaAndSizes uint32 DataDirectory [16]DataDirectory } ... But that is not always the case, there could be less no of entries (PE signed linux kernel for example): $ sudo pev /boot/vmlinuz-4.15.0-47-generic .... Data-dictionary entries: 6 .... In such case, the parsing gives incorrect results. This changes aims to fix that by: 1. Determining type of optional header by looking at header magic instead of size 2. Parsing optional header in 2 steps: a. Fixed part b. Variable data directories part Testing: 1. Fixed existing test cases to reflect the change 2. Added new file (modified linux kernel image) which has smaller number of data directories Fixes golang#32126 Change-Id: Iee56ecc4369a0e75a4be805e7cb8555c7d81ae2f Reviewed-on: https://go-review.googlesource.com/c/go/+/177959 Run-TryBot: Alex Brainman <alex.brainman@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
The debug/pe package assumes there are always 16 entries in DataDirectory in OptionalHeader32/64 ref pe.go: ... NumberOfRvaAndSizes uint32 DataDirectory [16]DataDirectory } ... But that is not always the case, there could be less no of entries (PE signed linux kernel for example): $ sudo pev /boot/vmlinuz-4.15.0-47-generic .... Data-dictionary entries: 6 .... In such case, the parsing gives incorrect results. This changes aims to fix that by: 1. Determining type of optional header by looking at header magic instead of size 2. Parsing optional header in 2 steps: a. Fixed part b. Variable data directories part Testing: 1. Fixed existing test cases to reflect the change 2. Added new file (modified linux kernel image) which has smaller number of data directories Fixes golang#32126 Change-Id: Iee56ecc4369a0e75a4be805e7cb8555c7d81ae2f Reviewed-on: https://go-review.googlesource.com/c/go/+/177959 Run-TryBot: Alex Brainman <alex.brainman@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes it does! The issue is present is current master.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I was trying to parse a PE file (linux Kernel to be specific) using debug/pe package.
A toy program for the same would look like:
What did you expect to see?
I would like to see parsed optional header.
What did you see instead?
Optional header turned out to be nil.
What is actual bug?
The debug/pe package assumes there are always 16 entries in [16]DataDirectory in OptionalHeader32/64 (ref pe.go)
NumberOfRvaAndSizes uint32
DataDirectory [16]DataDirectory
}
But that is not always the case, there could be less no of entries (PE signed linux kernel for example):
prashant@Pra-Work:~$ sudo pev /boot/vmlinuz-4.15.0-47-generic
....
Data-dictionary entries: 6
....
In such case, the parsing gives incorrect results.
The text was updated successfully, but these errors were encountered: