archive/tar: add missing error checks to Reader.Next

A recursive call to Reader.Next did not check the error before
trying to use the result, leading to a nil pointer panic.
This specific CL addresses the immediate issue, which is the panic,
but does not solve the root issue, which is due to an integer
overflow in the base-256 parser.

Updates #12435

Change-Id: Ia908671f0f411a409a35e24f2ebf740d46734072
Reviewed-on: https://go-review.googlesource.com/15437
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 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 67db71540c508ee4ababd1d4f531ff1d2a46935f..f38f8c8ce607f64235e9781b58493e907a7445fe 100644 (file)
--- a/src/archive/tar/reader.go
+++ b/src/archive/tar/reader.go
@@ -165,18 +165,24 @@ func (tr *Reader) Next() (*Header, error) {
                if err != nil {
                        return nil, err
                }
-               hdr, err := tr.Next()
+               hdr, tr.err = tr.Next()
+               if tr.err != nil {
+                       return nil, tr.err
+               }
                hdr.Name = cString(realname)
-               return hdr, err
+               return hdr, nil
        case TypeGNULongLink:
                // We have a GNU long link header.
                realname, err := ioutil.ReadAll(tr)
                if err != nil {
                        return nil, err
                }
-               hdr, err := tr.Next()
+               hdr, tr.err = tr.Next()
+               if tr.err != nil {
+                       return nil, tr.err
+               }
                hdr.Linkname = cString(realname)
-               return hdr, err
+               return hdr, nil
        }
        return hdr, tr.err
 }

diff --git a/src/archive/tar/reader_test.go b/src/archive/tar/reader_test.go
index 4d065a9591c67c8c0334c4d2dd6edb497ef18a2e..604d13f57b4224cbb44ab4defae8aa9616618e01 100644 (file)
--- a/src/archive/tar/reader_test.go
+++ b/src/archive/tar/reader_test.go
@@ -300,6 +300,14 @@ var untarTests = []*untarTest{
                file: "testdata/issue11169.tar",
                // TODO(dsnet): Currently the library does not detect that this file is
                // malformed. Instead it incorrectly believes that file just ends.
+               // At least the library doesn't crash anymore.
+               // err:  ErrHeader,
+       },
+       {
+               file: "testdata/issue12435.tar",
+               // TODO(dsnet): Currently the library does not detect that this file is
+               // malformed. Instead, it incorrectly believes that file just ends.
+               // At least the library doesn't crash anymore.
                // err:  ErrHeader,
        },
 }

diff --git a/src/archive/tar/testdata/issue12435.tar b/src/archive/tar/testdata/issue12435.tar
new file mode 100644 (file)
index 0000000..3542dd8
Binary files /dev/null and b/src/archive/tar/testdata/issue12435.tar differ