debug/elf: don't check section size

[gregory-m]

What version of Go are you using (go version)?

go1.12.7
also reproducible on tip

Does this issue reproduce with the latest release?

yes

What did you do?

https://play.golang.org/p/_5bUxWBh14g

package main

import (
	"debug/elf"
	"fmt"
	"strings"
)

func main() {
	data := "\u007fELF\x02\x01\x010000000000000" +
		"\x010000000000000000000" +
		"\x02\x00\x00\x00\x00\x00\x00\x0000000000\x00\x00\x00\x00" +
		"000\x0000"

	_, err := elf.NewFile(strings.NewReader(data))
	if err != nil {
		fmt.Println(err)
	}
}

What did you expect to see?

no panic

What did you see instead?

panic: runtime error: makeslice: len out of range

goroutine 1 [running]:
debug/elf.(*Section).Data(0x44b2c0, 0x43e300, 0x7f6900, 0x206590, 0x109440, 0x7fb880, 0x0, 0x0)
	/usr/local/go/src/debug/elf/file.go:105 +0x40
debug/elf.NewFile(0x15d640, 0x43e2e0, 0x43e2e0, 0x1293, 0x432070, 0x11)
	/usr/local/go/src/debug/elf/file.go:448 +0x1660
main.main()
	/tmp/sandbox898898450/prog.go:15 +0xa0

Fund with https://github.com/dvyukov/go-fuzz

[odeke-em]

Thank you for this report @gregory-m!

Kindly paging @tklauser @heschik @aclements.

[ianlancetaylor]

I'm not really sure how hard debug/elf should work to handle invalid input files.

[gregory-m]

I have same question as @ianlancetaylor, because I have 2 more panics in different places.

[rockdaboot]

I'm not really sure how hard debug/elf should work to handle invalid input files.

Here is at least one reason why NewFile() should not crash because of invalidated input.

We have a service that extracts BuildIDs from ELF files provided from different sources (distro packages or arbitrary upload from user).
For this we use NewFile() to load the ELF headers and then extract the BuildID from ".note.gnu.build-id" (or for kernel object we use ".notes").

We already saw some crashes of the service (luckily, it automatically recovers =)).

Now I ran into this again while experimenting with the Go fuzzer.
Here are some details, let me know if you prefer a separate issue.

Go built from commit 7e72d38.
"fatal error: runtime: out of memory" happens at

        /home/tim/src/go/src/debug/elf/file.go:470 +0x1351 fp=0xc0002b76f8 sp=0xc0002b7448 pc=0x5da291

The input file is
e088d7f758b82c73.gz

In line 470 (f.Sections = make([]*Section, shnum)), shnum is 4829504426752.

This crash prevents the fuzzer from going on and finding more/other issues.

[ZekeLu]

The issue reported on the first comment has been fixed on tip. Now it returns an error invalid ELF shentsize '0' in record at byte 0x0 instead of panics.

As for the issue reported by @rockdaboot, maybe we can fix it like this:

diff --git a/src/debug/elf/file.go b/src/debug/elf/file.go
index 708980bc1c..f4592f2cf2 100644
--- a/src/debug/elf/file.go
+++ b/src/debug/elf/file.go
@@ -467,8 +467,12 @@ func NewFile(r io.ReaderAt) (*File, error) {
    }
 
    // Read section headers
-   f.Sections = make([]*Section, shnum)
-   names := make([]uint32, shnum)
+   safeShnum := 0xfffff
+   if safeShnum > shnum {
+       safeShnum = shnum
+   }
+   f.Sections = make([]*Section, 0, safeShnum)
+   names := make([]uint32, 0, safeShnum)
    for i := 0; i < shnum; i++ {
        off := shoff + int64(i)*int64(shentsize)
        sr.Seek(off, seekStart)
@@ -479,7 +483,7 @@ func NewFile(r io.ReaderAt) (*File, error) {
            if err := binary.Read(sr, f.ByteOrder, sh); err != nil {
                return nil, err
            }
-           names[i] = sh.Name
+           names = append(names, sh.Name)
            s.SectionHeader = SectionHeader{
                Type:      SectionType(sh.Type),
                Flags:     SectionFlag(sh.Flags),
@@ -496,7 +500,7 @@ func NewFile(r io.ReaderAt) (*File, error) {
            if err := binary.Read(sr, f.ByteOrder, sh); err != nil {
                return nil, err
            }
-           names[i] = sh.Name
+           names = append(names, sh.Name)
            s.SectionHeader = SectionHeader{
                Type:      SectionType(sh.Type),
                Flags:     SectionFlag(sh.Flags),
@@ -544,7 +548,7 @@ func NewFile(r io.ReaderAt) (*File, error) {
            }
        }
 
-       f.Sections[i] = s
+       f.Sections = append(f.Sections, s)
    }
 
    if len(f.Sections) == 0 {

@ianlancetaylor I will send a CL if you're okay with this solution.

[rockdaboot]

@ZekeLu Thanks, your suggestion looks much better than the hack that I applied here to continue with fuzzing 😄 :

diff --git a/src/debug/elf/file.go b/src/debug/elf/file.go
index 708980bc1c..2b1aaa3938 100644
--- a/src/debug/elf/file.go
+++ b/src/debug/elf/file.go
@@ -466,6 +466,13 @@ func NewFile(r io.ReaderAt) (*File, error) {
                return nil, &FormatError{0, "invalid ELF shentsize", shentsize}
        }
 
+       // Plausibility check to avoid an unbounded memory allocation.
+       var test [1]byte
+       sr.Seek(int64(shnum) * 40, seekStart)
+       if err := binary.Read(sr, f.ByteOrder, test); err != nil {
+               return nil, err
+       }
+
        // Read section headers
        f.Sections = make([]*Section, shnum)
        names := make([]uint32, shnum)

[ianlancetaylor]

We've more or less decided that debug/elf should do at least minimal work to avoid panicking on invalid files. See #47653 and several recent commits to the debug/elf package.

@ZekeLu Since ELF files can have enormous numbers of sections, we should probably use internal/saferio.SliceCap. In any case, fixing that code is fine. Thanks.

[ZekeLu]

Ah, I totally forgot that we already have internal/saferio.SliceCap. I will send a CL utilized it soon. Thank you!

[gopherbot]

Change https://go.dev/cl/445076 mentions this issue: debug/elf: use saferio.SliceCap when decoding ELF sections

[ZekeLu]

The listed issues have been fixed.

I have 2 more panics in different places.

@gregory-m Can you confirm whether the 2 more panics can still be reproduced on go tip?

@gopherbot Please add label WaitingForInfo.

[gregory-m]

With CL445076 I can't reproduce it.
Thanks for fixing it.