debug/macho, internal/saferio: limit slice allocation

Don't allocate slices that are too large; choose a smaller capacity
and build the slice using append. Use this in debug/macho to avoid
over-allocating if a fat header is incorrect.

No debug/macho test case because the problem can only happen for
invalid data. Let the fuzzer find cases like this.

For #47653
Fixes #52523

Change-Id: I372c9cdbdda8626a3225e79d713650beb350ebc7
Reviewed-on: https://go-review.googlesource.com/c/go/+/413874
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>

diff --git a/src/debug/macho/fat.go b/src/debug/macho/fat.go
index 6bd730dc0bb8ff631463323fc45d8b4d84f53d99..775beaf12ce43319f47e5c0f397dd0479343c47f 100644 (file)
--- a/src/debug/macho/fat.go
+++ b/src/debug/macho/fat.go
@@ -7,6 +7,7 @@ package macho
 import (
        "encoding/binary"
        "fmt"
+       "internal/saferio"
        "io"
        "os"
 )
@@ -85,9 +86,13 @@ func NewFatFile(r io.ReaderAt) (*FatFile, error) {
 
        // Following the fat_header comes narch fat_arch structs that index
        // Mach-O images further in the file.
-       ff.Arches = make([]FatArch, narch)
+       c := saferio.SliceCap(FatArch{}, uint64(narch))
+       if c < 0 {
+               return nil, &FormatError{offset, "too many images", nil}
+       }
+       ff.Arches = make([]FatArch, 0, c)
        for i := uint32(0); i < narch; i++ {
-               fa := &ff.Arches[i]
+               var fa FatArch
                err = binary.Read(sr, binary.BigEndian, &fa.FatArchHeader)
                if err != nil {
                        return nil, &FormatError{offset, "invalid fat_arch header", nil}
@@ -115,6 +120,8 @@ func NewFatFile(r io.ReaderAt) (*FatFile, error) {
                                return nil, &FormatError{offset, fmt.Sprintf("Mach-O type for architecture #%d (type=%#x) does not match first (type=%#x)", i, fa.Type, machoType), nil}
                        }
                }
+
+               ff.Arches = append(ff.Arches, fa)
        }
 
        return &ff, nil

diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index e911f2f3411a4aabca132b7a811d5c5e66261eb4..061345f64b69180751caacc90e7fcad877d51dcb 100644 (file)
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -123,9 +123,6 @@ var depsRules = `
 
        unicode !< strconv;
 
-       io
-       < internal/saferio;
-
        # STR is basic string and buffer manipulation.
        RUNTIME, io, unicode/utf8, unicode/utf16, unicode
        < bytes, strings
@@ -242,6 +239,9 @@ var depsRules = `
        encoding/binary, regexp
        < index/suffixarray;
 
+       io, reflect
+       < internal/saferio;
+
        # executable parsing
        FMT, encoding/binary, compress/zlib, internal/saferio
        < runtime/debug

diff --git a/src/internal/saferio/io.go b/src/internal/saferio/io.go
index 019216f35277b14dd62ee9a0a2bd561a5b2819b7..0361011e95a5eb77fa4615952f95528bbb086961 100644 (file)
--- a/src/internal/saferio/io.go
+++ b/src/internal/saferio/io.go
@@ -9,7 +9,10 @@
 // untrustworthy attacker.
 package saferio
 
-import "io"
+import (
+       "io"
+       "reflect"
+)
 
 // chunk is an arbitrary limit on how much memory we are willing
 // to allocate without concern.
@@ -91,3 +94,27 @@ func ReadDataAt(r io.ReaderAt, n uint64, off int64) ([]byte, error) {
        }
        return buf, nil
 }
+
+// SliceCap returns the capacity to use when allocating a slice.
+// After the slice is allocated with the capacity, it should be
+// built using append. This will avoid allocating too much memory
+// if the capacity is large and incorrect.
+//
+// A negative result means that the value is always too big.
+//
+// The element type is described by passing a value of that type.
+// This would ideally use generics, but this code is built with
+// the bootstrap compiler which need not support generics.
+func SliceCap(v any, c uint64) int {
+       if int64(c) < 0 || c != uint64(int(c)) {
+               return -1
+       }
+       size := reflect.TypeOf(v).Size()
+       if uintptr(c)*size > chunk {
+               c = uint64(chunk / size)
+               if c == 0 {
+                       c = 1
+               }
+       }
+       return int(c)
+}

diff --git a/src/internal/saferio/io_test.go b/src/internal/saferio/io_test.go
index 301b798834d677a2a53279065c40e6fce08ee273..9214e735c216a6efc2f7bc3534475818ac3093e9 100644 (file)
--- a/src/internal/saferio/io_test.go
+++ b/src/internal/saferio/io_test.go
@@ -81,3 +81,28 @@ func TestReadDataAt(t *testing.T) {
                }
        })
 }
+
+func TestSliceCap(t *testing.T) {
+       t.Run("small", func(t *testing.T) {
+               c := SliceCap(0, 10)
+               if c != 10 {
+                       t.Errorf("got capacity %d, want %d", c, 10)
+               }
+       })
+
+       t.Run("large", func(t *testing.T) {
+               c := SliceCap(byte(0), 1<<30)
+               if c < 0 {
+                       t.Error("SliceCap failed unexpectedly")
+               } else if c == 1<<30 {
+                       t.Errorf("got capacity %d which is too high", c)
+               }
+       })
+
+       t.Run("maxint", func(t *testing.T) {
+               c := SliceCap(byte(0), 1<<63)
+               if c >= 0 {
+                       t.Errorf("SliceCap returned %d, expected failure", c)
+               }
+       })
+}