Skip to content

Commit cdb3789

Browse files
ianlancetaylorgopherbot
authored andcommitted
debug/pe, internal/saferio: use saferio to read PE section data
For #47653 Fixes #53189 Change-Id: If35b968fc53e4c96b18964cfb020cdc003b881bf Reviewed-on: https://go-review.googlesource.com/c/go/+/412014 Reviewed-by: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> Reviewed-by: Alex Brainman <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent 9e6cd39 commit cdb3789

File tree

3 files changed

+87
-6
lines changed

3 files changed

+87
-6
lines changed

src/debug/pe/section.go

+2-6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package pe
77
import (
88
"encoding/binary"
99
"fmt"
10+
"internal/saferio"
1011
"io"
1112
"strconv"
1213
)
@@ -97,12 +98,7 @@ type Section struct {
9798

9899
// Data reads and returns the contents of the PE section s.
99100
func (s *Section) Data() ([]byte, error) {
100-
dat := make([]byte, s.sr.Size())
101-
n, err := s.sr.ReadAt(dat, 0)
102-
if n == len(dat) {
103-
err = nil
104-
}
105-
return dat[0:n], err
101+
return saferio.ReadDataAt(s.sr, uint64(s.sr.Size()), 0)
106102
}
107103

108104
// Open returns a new ReadSeeker reading the PE section s.

src/internal/saferio/io.go

+41
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,44 @@ func ReadData(r io.Reader, n uint64) ([]byte, error) {
5050
}
5151
return buf, nil
5252
}
53+
54+
// ReadDataAt reads n bytes from the input stream at off, but avoids
55+
// allocating all n bytes if n is large. This avoids crashing the program
56+
// by allocating all n bytes in cases where n is incorrect.
57+
func ReadDataAt(r io.ReaderAt, n uint64, off int64) ([]byte, error) {
58+
if int64(n) < 0 || n != uint64(int(n)) {
59+
// n is too large to fit in int, so we can't allocate
60+
// a buffer large enough. Treat this as a read failure.
61+
return nil, io.ErrUnexpectedEOF
62+
}
63+
64+
if n < chunk {
65+
buf := make([]byte, n)
66+
_, err := r.ReadAt(buf, off)
67+
if err != nil {
68+
// io.SectionReader can return EOF for n == 0,
69+
// but for our purposes that is a success.
70+
if err != io.EOF || n > 0 {
71+
return nil, err
72+
}
73+
}
74+
return buf, nil
75+
}
76+
77+
var buf []byte
78+
buf1 := make([]byte, chunk)
79+
for n > 0 {
80+
next := n
81+
if next > chunk {
82+
next = chunk
83+
}
84+
_, err := r.ReadAt(buf1[:next], off)
85+
if err != nil {
86+
return nil, err
87+
}
88+
buf = append(buf, buf1[:next]...)
89+
n -= next
90+
off += int64(next)
91+
}
92+
return buf, nil
93+
}

src/internal/saferio/io_test.go

+44
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package saferio
66

77
import (
88
"bytes"
9+
"io"
910
"testing"
1011
)
1112

@@ -37,3 +38,46 @@ func TestReadData(t *testing.T) {
3738
}
3839
})
3940
}
41+
42+
func TestReadDataAt(t *testing.T) {
43+
const count = 100
44+
input := bytes.Repeat([]byte{'a'}, count)
45+
46+
t.Run("small", func(t *testing.T) {
47+
got, err := ReadDataAt(bytes.NewReader(input), count, 0)
48+
if err != nil {
49+
t.Fatal(err)
50+
}
51+
if !bytes.Equal(got, input) {
52+
t.Errorf("got %v, want %v", got, input)
53+
}
54+
})
55+
56+
t.Run("large", func(t *testing.T) {
57+
_, err := ReadDataAt(bytes.NewReader(input), 10<<30, 0)
58+
if err == nil {
59+
t.Error("large read succeeded unexpectedly")
60+
}
61+
})
62+
63+
t.Run("maxint", func(t *testing.T) {
64+
_, err := ReadDataAt(bytes.NewReader(input), 1<<62, 0)
65+
if err == nil {
66+
t.Error("large read succeeded unexpectedly")
67+
}
68+
})
69+
70+
t.Run("SectionReader", func(t *testing.T) {
71+
// Reading 0 bytes from an io.SectionReader at the end
72+
// of the section will return EOF, but ReadDataAt
73+
// should succeed and return 0 bytes.
74+
sr := io.NewSectionReader(bytes.NewReader(input), 0, 0)
75+
got, err := ReadDataAt(sr, 0, 0)
76+
if err != nil {
77+
t.Fatal(err)
78+
}
79+
if len(got) > 0 {
80+
t.Errorf("got %d bytes, expected 0", len(got))
81+
}
82+
})
83+
}

0 commit comments

Comments
 (0)