archive/tar: fix numeric overflow issues in readGNUSparseMap0x1

Motivation:
* The logic to verify the numEntries can overflow and incorrectly
pass, allowing a malicious file to allocate arbitrary memory.
* The use of strconv.ParseInt does not set the integer precision
to 64bit, causing this code to work incorrectly on 32bit machines.

Change-Id: I1b1571a750a84f2dde97cc329ed04fe2342aaa60
Reviewed-on: https://go-review.googlesource.com/15173
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/archive/tar/reader.go b/src/archive/tar/reader.go
index f38f8c8ce607f64235e9781b58493e907a7445fe..b2c45fd38841d026e325291e8903d615cbee1ac7 100644 (file)
--- a/src/archive/tar/reader.go
+++ b/src/archive/tar/reader.go
@@ -718,40 +718,37 @@ func readGNUSparseMap1x0(r io.Reader) ([]sparseEntry, error) {
        return sp, nil
 }
 
-// readGNUSparseMap0x1 reads the sparse map as stored in GNU's PAX sparse format version 0.1.
-// The sparse map is stored in the PAX headers.
-func readGNUSparseMap0x1(headers map[string]string) ([]sparseEntry, error) {
-       // Get number of entries
-       numEntriesStr, ok := headers[paxGNUSparseNumBlocks]
-       if !ok {
-               return nil, ErrHeader
-       }
-       numEntries, err := strconv.ParseInt(numEntriesStr, 10, 0)
-       if err != nil {
+// readGNUSparseMap0x1 reads the sparse map as stored in GNU's PAX sparse format
+// version 0.1. The sparse map is stored in the PAX headers.
+func readGNUSparseMap0x1(extHdrs map[string]string) ([]sparseEntry, error) {
+       // Get number of entries.
+       // Use integer overflow resistant math to check this.
+       numEntriesStr := extHdrs[paxGNUSparseNumBlocks]
+       numEntries, err := strconv.ParseInt(numEntriesStr, 10, 0) // Intentionally parse as native int
+       if err != nil || numEntries < 0 || int(2*numEntries) < int(numEntries) {
                return nil, ErrHeader
        }
 
-       sparseMap := strings.Split(headers[paxGNUSparseMap], ",")
-
-       // There should be two numbers in sparseMap for each entry
+       // There should be two numbers in sparseMap for each entry.
+       sparseMap := strings.Split(extHdrs[paxGNUSparseMap], ",")
        if int64(len(sparseMap)) != 2*numEntries {
                return nil, ErrHeader
        }
 
-       // Loop through the entries in the sparse map
+       // Loop through the entries in the sparse map.
+       // numEntries is trusted now.
        sp := make([]sparseEntry, 0, numEntries)
        for i := int64(0); i < numEntries; i++ {
-               offset, err := strconv.ParseInt(sparseMap[2*i], 10, 0)
+               offset, err := strconv.ParseInt(sparseMap[2*i], 10, 64)
                if err != nil {
                        return nil, ErrHeader
                }
-               numBytes, err := strconv.ParseInt(sparseMap[2*i+1], 10, 0)
+               numBytes, err := strconv.ParseInt(sparseMap[2*i+1], 10, 64)
                if err != nil {
                        return nil, ErrHeader
                }
                sp = append(sp, sparseEntry{offset: offset, numBytes: numBytes})
        }
-
        return sp, nil
 }
 

diff --git a/src/archive/tar/reader_test.go b/src/archive/tar/reader_test.go
index 604d13f57b4224cbb44ab4defae8aa9616618e01..4a6d1a9e9f275b247304af63976859c9145541e9 100644 (file)
--- a/src/archive/tar/reader_test.go
+++ b/src/archive/tar/reader_test.go
@@ -680,23 +680,78 @@ func TestSparseFileReader(t *testing.T) {
 }
 
 func TestReadGNUSparseMap0x1(t *testing.T) {
-       headers := map[string]string{
-               paxGNUSparseNumBlocks: "4",
-               paxGNUSparseMap:       "0,5,10,5,20,5,30,5",
-       }
-       expected := []sparseEntry{
-               {offset: 0, numBytes: 5},
-               {offset: 10, numBytes: 5},
-               {offset: 20, numBytes: 5},
-               {offset: 30, numBytes: 5},
-       }
+       const (
+               maxUint = ^uint(0)
+               maxInt  = int(maxUint >> 1)
+       )
+       var (
+               big1 = fmt.Sprintf("%d", int64(maxInt))
+               big2 = fmt.Sprintf("%d", (int64(maxInt)/2)+1)
+               big3 = fmt.Sprintf("%d", (int64(maxInt) / 3))
+       )
 
-       sp, err := readGNUSparseMap0x1(headers)
-       if err != nil {
-               t.Errorf("Unexpected error: %v", err)
-       }
-       if !reflect.DeepEqual(sp, expected) {
-               t.Errorf("Incorrect sparse map: got %v, wanted %v", sp, expected)
+       var vectors = []struct {
+               extHdrs   map[string]string // Input data
+               sparseMap []sparseEntry     // Expected sparse entries to be outputted
+               err       error             // Expected errors that may be raised
+       }{{
+               extHdrs: map[string]string{paxGNUSparseNumBlocks: "-4"},
+               err:     ErrHeader,
+       }, {
+               extHdrs: map[string]string{paxGNUSparseNumBlocks: "fee "},
+               err:     ErrHeader,
+       }, {
+               extHdrs: map[string]string{
+                       paxGNUSparseNumBlocks: big1,
+                       paxGNUSparseMap:       "0,5,10,5,20,5,30,5",
+               },
+               err: ErrHeader,
+       }, {
+               extHdrs: map[string]string{
+                       paxGNUSparseNumBlocks: big2,
+                       paxGNUSparseMap:       "0,5,10,5,20,5,30,5",
+               },
+               err: ErrHeader,
+       }, {
+               extHdrs: map[string]string{
+                       paxGNUSparseNumBlocks: big3,
+                       paxGNUSparseMap:       "0,5,10,5,20,5,30,5",
+               },
+               err: ErrHeader,
+       }, {
+               extHdrs: map[string]string{
+                       paxGNUSparseNumBlocks: "4",
+                       paxGNUSparseMap:       "0.5,5,10,5,20,5,30,5",
+               },
+               err: ErrHeader,
+       }, {
+               extHdrs: map[string]string{
+                       paxGNUSparseNumBlocks: "4",
+                       paxGNUSparseMap:       "0,5.5,10,5,20,5,30,5",
+               },
+               err: ErrHeader,
+       }, {
+               extHdrs: map[string]string{
+                       paxGNUSparseNumBlocks: "4",
+                       paxGNUSparseMap:       "0,fewafewa.5,fewafw,5,20,5,30,5",
+               },
+               err: ErrHeader,
+       }, {
+               extHdrs: map[string]string{
+                       paxGNUSparseNumBlocks: "4",
+                       paxGNUSparseMap:       "0,5,10,5,20,5,30,5",
+               },
+               sparseMap: []sparseEntry{{0, 5}, {10, 5}, {20, 5}, {30, 5}},
+       }}
+
+       for i, v := range vectors {
+               sp, err := readGNUSparseMap0x1(v.extHdrs)
+               if !reflect.DeepEqual(sp, v.sparseMap) && !(len(sp) == 0 && len(v.sparseMap) == 0) {
+                       t.Errorf("test %d, readGNUSparseMap0x1(...): got %v, want %v", i, sp, v.sparseMap)
+               }
+               if err != v.err {
+                       t.Errorf("test %d, unexpected error: got %v, want %v", i, err, v.err)
+               }
        }
 }