debug/pe: return error on reading from section with uninitialized data

A section with uninitialized data contains no bytes and occupies
no space in the file. This change makes it return an error on reading
from this section so that it will force the caller to check for
a section with uninitialized data.

This is the debug/pe version of CL 429601.

This will break programs that expect a byte slice with the length
described by the SizeOfRawData field. There are two reasons to
introduce this breaking change: 1) uninitialized data is uninitialized
and there is no reason to allocate memory for it; 2) it could result
in an OOM if the file is corrupted and has a large invalid SizeOfRawData.

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

For #47653
Fixes #59817

Change-Id: I1ae94e9508f803b37926275d9a571f724a09af9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/488475
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: kortschak <dan@kortschak.io>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>

diff --git a/src/debug/pe/file.go b/src/debug/pe/file.go
index de4bb9b7366b0db8b2a89f71c9ce87b3f1ad29b3..06c160105f57f7b67ab8dafabd41d4ce77349e1b 100644 (file)
--- a/src/debug/pe/file.go
+++ b/src/debug/pe/file.go
@@ -20,6 +20,7 @@ import (
        "compress/zlib"
        "debug/dwarf"
        "encoding/binary"
+       "errors"
        "fmt"
        "io"
        "os"
@@ -165,7 +166,7 @@ func NewFile(r io.ReaderAt) (*File, error) {
                }
                r2 := r
                if sh.PointerToRawData == 0 { // .bss must have all 0s
-                       r2 = zeroReaderAt{}
+                       r2 = &nobitsSectionReader{}
                }
                s.sr = io.NewSectionReader(r2, int64(s.SectionHeader.Offset), int64(s.SectionHeader.Size))
                s.ReaderAt = s.sr
@@ -182,15 +183,10 @@ func NewFile(r io.ReaderAt) (*File, error) {
        return f, nil
 }
 
-// zeroReaderAt is ReaderAt that reads 0s.
-type zeroReaderAt struct{}
+type nobitsSectionReader struct{}
 
-// ReadAt writes len(p) 0s into p.
-func (w zeroReaderAt) ReadAt(p []byte, off int64) (n int, err error) {
-       for i := range p {
-               p[i] = 0
-       }
-       return len(p), nil
+func (*nobitsSectionReader) ReadAt(p []byte, off int64) (n int, err error) {
+       return 0, errors.New("unexpected read from section with uninitialized data")
 }
 
 // getString extracts a string from symbol string table.
@@ -363,6 +359,9 @@ func (f *File) ImportedSymbols() ([]string, error) {
        var ds *Section
        ds = nil
        for _, s := range f.Sections {
+               if s.Offset == 0 {
+                       continue
+               }
                // We are using distance between s.VirtualAddress and idd.VirtualAddress
                // to avoid potential overflow of uint32 caused by addition of s.VirtualSize
                // to s.VirtualAddress.

diff --git a/src/debug/pe/file_test.go b/src/debug/pe/file_test.go
index 5368e08ad72342e22b220900a73d9c0c316688f0..3d960ab7f36b84a08b4df3568d6f40f8e4920b61 100644 (file)
--- a/src/debug/pe/file_test.go
+++ b/src/debug/pe/file_test.go
@@ -511,17 +511,9 @@ main(void)
        if bss == nil {
                t.Fatal("could not find .bss section")
        }
-       data, err := bss.Data()
-       if err != nil {
-               t.Fatal(err)
-       }
-       if len(data) == 0 {
-               t.Fatalf("%s file .bss section cannot be empty", objpath)
-       }
-       for _, b := range data {
-               if b != 0 {
-                       t.Fatalf(".bss section has non zero bytes: %v", data)
-               }
+       // We expect an error from bss.Data, as there are no contents.
+       if _, err := bss.Data(); err == nil {
+               t.Error("bss.Data succeeded, expected error")
        }
 }
 

diff --git a/src/debug/pe/section.go b/src/debug/pe/section.go
index fabb47af2eae175964e91f4c5869ef3a2c552f85..70d0c220ce2c7a7e929afa3bbc50fc2c2fa064e3 100644 (file)
--- a/src/debug/pe/section.go
+++ b/src/debug/pe/section.go
@@ -97,11 +97,17 @@ type Section struct {
 }
 
 // Data reads and returns the contents of the PE section s.
+//
+// If s.Offset is 0, the section has no contents,
+// and Data will always return a non-nil error.
 func (s *Section) Data() ([]byte, error) {
        return saferio.ReadDataAt(s.sr, uint64(s.Size), 0)
 }
 
 // Open returns a new ReadSeeker reading the PE section s.
+//
+// If s.Offset is 0, the section has no contents, and all calls
+// to the returned reader will return a non-nil error.
 func (s *Section) Open() io.ReadSeeker {
        return io.NewSectionReader(s.sr, 0, 1<<63-1)
 }