Skip to content

Commit 7adfa82

Browse files
ianlancetaylorgopherbot
authored andcommitted
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 <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Tobias Klauser <[email protected]>
1 parent 7142480 commit 7adfa82

File tree

4 files changed

+65
-6
lines changed

4 files changed

+65
-6
lines changed

src/debug/macho/fat.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package macho
77
import (
88
"encoding/binary"
99
"fmt"
10+
"internal/saferio"
1011
"io"
1112
"os"
1213
)
@@ -85,9 +86,13 @@ func NewFatFile(r io.ReaderAt) (*FatFile, error) {
8586

8687
// Following the fat_header comes narch fat_arch structs that index
8788
// Mach-O images further in the file.
88-
ff.Arches = make([]FatArch, narch)
89+
c := saferio.SliceCap(FatArch{}, uint64(narch))
90+
if c < 0 {
91+
return nil, &FormatError{offset, "too many images", nil}
92+
}
93+
ff.Arches = make([]FatArch, 0, c)
8994
for i := uint32(0); i < narch; i++ {
90-
fa := &ff.Arches[i]
95+
var fa FatArch
9196
err = binary.Read(sr, binary.BigEndian, &fa.FatArchHeader)
9297
if err != nil {
9398
return nil, &FormatError{offset, "invalid fat_arch header", nil}
@@ -115,6 +120,8 @@ func NewFatFile(r io.ReaderAt) (*FatFile, error) {
115120
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}
116121
}
117122
}
123+
124+
ff.Arches = append(ff.Arches, fa)
118125
}
119126

120127
return &ff, nil

src/go/build/deps_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,6 @@ var depsRules = `
123123
124124
unicode !< strconv;
125125
126-
io
127-
< internal/saferio;
128-
129126
# STR is basic string and buffer manipulation.
130127
RUNTIME, io, unicode/utf8, unicode/utf16, unicode
131128
< bytes, strings
@@ -242,6 +239,9 @@ var depsRules = `
242239
encoding/binary, regexp
243240
< index/suffixarray;
244241
242+
io, reflect
243+
< internal/saferio;
244+
245245
# executable parsing
246246
FMT, encoding/binary, compress/zlib, internal/saferio
247247
< runtime/debug

src/internal/saferio/io.go

+28-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
// untrustworthy attacker.
1010
package saferio
1111

12-
import "io"
12+
import (
13+
"io"
14+
"reflect"
15+
)
1316

1417
// chunk is an arbitrary limit on how much memory we are willing
1518
// to allocate without concern.
@@ -91,3 +94,27 @@ func ReadDataAt(r io.ReaderAt, n uint64, off int64) ([]byte, error) {
9194
}
9295
return buf, nil
9396
}
97+
98+
// SliceCap returns the capacity to use when allocating a slice.
99+
// After the slice is allocated with the capacity, it should be
100+
// built using append. This will avoid allocating too much memory
101+
// if the capacity is large and incorrect.
102+
//
103+
// A negative result means that the value is always too big.
104+
//
105+
// The element type is described by passing a value of that type.
106+
// This would ideally use generics, but this code is built with
107+
// the bootstrap compiler which need not support generics.
108+
func SliceCap(v any, c uint64) int {
109+
if int64(c) < 0 || c != uint64(int(c)) {
110+
return -1
111+
}
112+
size := reflect.TypeOf(v).Size()
113+
if uintptr(c)*size > chunk {
114+
c = uint64(chunk / size)
115+
if c == 0 {
116+
c = 1
117+
}
118+
}
119+
return int(c)
120+
}

src/internal/saferio/io_test.go

+25
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,28 @@ func TestReadDataAt(t *testing.T) {
8181
}
8282
})
8383
}
84+
85+
func TestSliceCap(t *testing.T) {
86+
t.Run("small", func(t *testing.T) {
87+
c := SliceCap(0, 10)
88+
if c != 10 {
89+
t.Errorf("got capacity %d, want %d", c, 10)
90+
}
91+
})
92+
93+
t.Run("large", func(t *testing.T) {
94+
c := SliceCap(byte(0), 1<<30)
95+
if c < 0 {
96+
t.Error("SliceCap failed unexpectedly")
97+
} else if c == 1<<30 {
98+
t.Errorf("got capacity %d which is too high", c)
99+
}
100+
})
101+
102+
t.Run("maxint", func(t *testing.T) {
103+
c := SliceCap(byte(0), 1<<63)
104+
if c >= 0 {
105+
t.Errorf("SliceCap returned %d, expected failure", c)
106+
}
107+
})
108+
}

0 commit comments

Comments
 (0)