From 4ae2bf3c0ce03e44c073a95c4e0333adedd280e9 Mon Sep 17 00:00:00 2001
From: Lorenz Bauer <lmb@cloudflare.com>
Date: Fri, 27 Nov 2020 11:20:45 +0000
Subject: [PATCH] internal: add safe ELF file wrapper

Fuzzing the ELF loader isn't very succesful since it keeps crashing
in debug/elf. Most of the time the culprit is a call to elf.Section.Data().
This method allocates a buffer with a size taken from the ELF, which
can lead to outlandishly large allocations. It's not clear how to validate
elf.Section.Size since the code that creates the section doesn't know
the total length of the ELF.

Instead, add a wrapper that catches panics due to ELF parsing, and turns
them into errors. This isn't a fool proof solution since the runtime
can still kill the process due to an OOM, but hopefully we will still
crash less overall.

See https://github.com/golang/go/issues/33121
---
 elf_reader.go       |  4 ++--
 internal/btf/btf.go |  6 +++---
 internal/elf.go     | 52 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 5 deletions(-)
 create mode 100644 internal/elf.go

diff --git a/elf_reader.go b/elf_reader.go
index 38bb7dad3..8f64d858c 100644
--- a/elf_reader.go
+++ b/elf_reader.go
@@ -19,7 +19,7 @@ import (
 )
 
 type elfCode struct {
-	*elf.File
+	*internal.SafeELFFile
 	symbols           []elf.Symbol
 	symbolsPerSection map[elf.SectionIndex]map[uint64]elf.Symbol
 	license           string
@@ -43,7 +43,7 @@ func LoadCollectionSpec(file string) (*CollectionSpec, error) {
 
 // LoadCollectionSpecFromReader parses an ELF file into a CollectionSpec.
 func LoadCollectionSpecFromReader(rd io.ReaderAt) (*CollectionSpec, error) {
-	f, err := elf.NewFile(rd)
+	f, err := internal.NewSafeELFFile(rd)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/btf/btf.go b/internal/btf/btf.go
index 51f84d0db..57f2b7d10 100644
--- a/internal/btf/btf.go
+++ b/internal/btf/btf.go
@@ -53,7 +53,7 @@ type btfHeader struct {
 //
 // Returns a nil Spec and no error if no BTF was present.
 func LoadSpecFromReader(rd io.ReaderAt) (*Spec, error) {
-	file, err := elf.NewFile(rd)
+	file, err := internal.NewSafeELFFile(rd)
 	if err != nil {
 		return nil, err
 	}
@@ -109,7 +109,7 @@ func LoadSpecFromReader(rd io.ReaderAt) (*Spec, error) {
 	return spec, nil
 }
 
-func findBtfSections(file *elf.File) (*elf.Section, *elf.Section, map[string]uint32, error) {
+func findBtfSections(file *internal.SafeELFFile) (*elf.Section, *elf.Section, map[string]uint32, error) {
 	var (
 		btfSection    *elf.Section
 		btfExtSection *elf.Section
@@ -138,7 +138,7 @@ func findBtfSections(file *elf.File) (*elf.Section, *elf.Section, map[string]uin
 }
 
 func loadSpecFromVmlinux(rd io.ReaderAt) (*Spec, error) {
-	file, err := elf.NewFile(rd)
+	file, err := internal.NewSafeELFFile(rd)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/elf.go b/internal/elf.go
new file mode 100644
index 000000000..c3f9ea0f8
--- /dev/null
+++ b/internal/elf.go
@@ -0,0 +1,52 @@
+package internal
+
+import (
+	"debug/elf"
+	"fmt"
+	"io"
+)
+
+type SafeELFFile struct {
+	*elf.File
+}
+
+// NewSafeELFFile reads an ELF safely.
+//
+// Any panic during parsing is turned into an error. This is necessary since
+// there are a bunch of unfixed bugs in debug/elf.
+//
+// https://github.com/golang/go/issues?q=is%3Aissue+is%3Aopen+debug%2Felf+in%3Atitle
+func NewSafeELFFile(r io.ReaderAt) (safe *SafeELFFile, err error) {
+	defer func() {
+		r := recover()
+		if r == nil {
+			return
+		}
+
+		safe = nil
+		err = fmt.Errorf("reading ELF file panicked: %s", r)
+	}()
+
+	file, err := elf.NewFile(r)
+	if err != nil {
+		return nil, err
+	}
+
+	return &SafeELFFile{file}, nil
+}
+
+// Symbols is the safe version of elf.File.Symbols.
+func (se *SafeELFFile) Symbols() (syms []elf.Symbol, err error) {
+	defer func() {
+		r := recover()
+		if r == nil {
+			return
+		}
+
+		syms = nil
+		err = fmt.Errorf("reading ELF symbols panicked: %s", r)
+	}()
+
+	syms, err = se.File.Symbols()
+	return
+}