Closed catenacyber closed 1 year ago
Is this related to #45599? CC @ianlancetaylor @thanm
Note that #45599 was fixed about a month ago on tip. I see that you're running go.dev/play against tip, so maybe this is still an issue?
Yes this is an issue on tip, so likely similar to #45599 but different...
Change https://go.dev/cl/429601 mentions this issue: debug/elf: avoid using saferio to read data from SHT_NOBITS sections
The test case uncovers another issue: incorrect huge size (3349099659509720927 bytes) of the SHT_NOBITS
section. An SHT_NOBITS
section may have a nonzero size, but it occupies no space in the file. Since the section size is "conceptual" and the data is provided by a zeroReader
, saferio.ReadData
does not help in this case. In fact, saferio.ReadData
will make it worse because it will eat up the memory one small block by one small block and results in an OOM finally.
It seems that there is not way to tell whether the section size is valid. So https://go.dev/cl/429601 returns the section data by calling make([]byte, s.Size)
directly. When the size is huge, it panics with runtime error: makeslice: len out of range
fast.
I think a further fix is to set a limit for the size of the SHT_NOBITS
section and refuse to read from this section when the size is larger than the limit. But I can not find a documented limit for this section.
The docs at https://refspecs.linuxfoundation.org/LSB_2.1.0/LSB-Core-generic/LSB-Core-generic/elftypes.html have
Name Value Description SHT_NOBITS
0x8 A section of this type occupies no space in the file but otherwise resembles SHT_PROGBITS
. Although this section contains no bytes, thesh_offset
member contains the conceptual file offset.
I'd suggest that for this case we do
func (s *Section) Data() ([]byte, error) {
if s.Type == SHT_NOBITS {
return nil, nil
}
return saferio.ReadData(s.Open(), s.Size)
}
This reflects the wording from the docs in that no data is present, but the user can still obtain the sh_offset
through the s.Offset
field. The user can obviously also obtain the claimed size through s.Size
.
Change https://go.dev/cl/430155 mentions this issue: debug/elf: validate shstrndx
@kortschak I like this solution! See also the discussion in #18667.
Since the CL (https://go.dev/cl/375216) that introduced this behavior has been released since go1.18, it seems that it's too late to change the behavior now.
And this solution requires the caller to check the section type. If the caller would like to check the section type, it can avoid calling (*Section).Data
at all when the section type is SHT_NOBITS
[^1]. It seems that breaking the compatibility does not worth it.
Regarding the caller of (*Section).Data
. I found that debug/elf/file.go
should not call (*Section).Data
when the section type is SHT_NOBITS
. The testing elf file has an invalid shstrndx
pointing to the SHT_NOBITS section. According to https://refspecs.linuxfoundation.org/elf/gabi4+/ch4.eheader.html, shstrndx
should point to the section name string table, which has the type SHT_STRTAB
. I have sent another CL (https://go.dev/cl/430155) to validate shstrndx
, which will in turn prevent it from calling (*Section).Data
at the first place.
[^1]: here is one such caller: https://github.com/aclements/go-obj/blob/91d9e299b01bb7d48c3ab33b24cdabb3fec63885/obj/elf.go#L420-L454
@aarzilli Can you take a look at https://go.dev/cl/429601 ? The consensus there is to let (*Section).Data
return an error when it is an SHT_NOBITS section. Thank you!
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Run https://go.dev/play/p/yNYIA1Rditp?v=gotip
What did you expect to see?
The program finishing and printing Hello
What did you see instead?
Found by https://github.com/catenacyber/ngolo-fuzzing on oss-fuzz https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=51098