archive/tar: ensure input fits in octal field

The prior logic would over-write the NUL-terminator if the octal value
was long enough. In order to prevent this, we add a fitsInOctal function
that does the proper check.

The relevant USTAR specification about NUL-terminator is:
<<<
Each numeric field is terminated by one or more <space> or NUL characters.
>>>

Change-Id: I6fbc6e8fe71168727eea201925d0fe08d43116ac
Reviewed-on: https://go-review.googlesource.com/54432
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/archive/tar/strconv.go b/src/archive/tar/strconv.go
index 3a635834ff45c47ce62c9f432b996efd3d08b319..7629c43c65ac73daaa487068d83a96552e7ae95c 100644 (file)
--- a/src/archive/tar/strconv.go
+++ b/src/archive/tar/strconv.go
@@ -124,8 +124,14 @@ func (p *parser) parseNumeric(b []byte) int64 {
        return p.parseOctal(b)
 }
 
-// Write x into b, as binary (GNUtar/star extension).
+// formatNumeric encodes x into b using base-8 (octal) encoding if possible.
+// Otherwise it will attempt to use base-256 (binary) encoding.
 func (f *formatter) formatNumeric(b []byte, x int64) {
+       if fitsInOctal(len(b), x) {
+               f.formatOctal(b, x)
+               return
+       }
+
        if fitsInBase256(len(b), x) {
                for i := len(b) - 1; i >= 0; i-- {
                        b[i] = byte(x)
@@ -166,6 +172,13 @@ func (f *formatter) formatOctal(b []byte, x int64) {
        f.formatString(b, s)
 }
 
+// fitsInOctal reports whether the integer x fits in a field n-bytes long
+// using octal encoding with the appropriate NUL terminator.
+func fitsInOctal(n int, x int64) bool {
+       octBits := uint(n-1) * 3
+       return x >= 0 && (n >= 22 || x < 1<<octBits)
+}
+
 // parsePAXTime takes a string of the form %d.%d as described in the PAX
 // specification. Note that this implementation allows for negative timestamps,
 // which is allowed for by the PAX specification, but not always portable.

diff --git a/src/archive/tar/strconv_test.go b/src/archive/tar/strconv_test.go
index 36e9413de253cd22f7eb6349a1140869c86a7098..e2527dc61f7589d9914e04d9b6abd7f7802f36df 100644 (file)
--- a/src/archive/tar/strconv_test.go
+++ b/src/archive/tar/strconv_test.go
@@ -110,6 +110,25 @@ func TestFormatNumeric(t *testing.T) {
                want string
                ok   bool
        }{
+               // Test base-8 (octal) encoded values.
+               {0, "0\x00", true},
+               {7, "7\x00", true},
+               {8, "\x80\x08", true},
+               {077, "77\x00", true},
+               {0100, "\x80\x00\x40", true},
+               {0, "0000000\x00", true},
+               {0123, "0000123\x00", true},
+               {07654321, "7654321\x00", true},
+               {07777777, "7777777\x00", true},
+               {010000000, "\x80\x00\x00\x00\x00\x20\x00\x00", true},
+               {0, "00000000000\x00", true},
+               {000001234567, "00001234567\x00", true},
+               {076543210321, "76543210321\x00", true},
+               {012345670123, "12345670123\x00", true},
+               {077777777777, "77777777777\x00", true},
+               {0100000000000, "\x80\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00", true},
+               {math.MaxInt64, "777777777777777777777\x00", true},
+
                // Test base-256 (binary) encoded values.
                {-1, "\xff", true},
                {-1, "\xff\xff", true},
@@ -155,6 +174,45 @@ func TestFormatNumeric(t *testing.T) {
        }
 }
 
+func TestFitsInOctal(t *testing.T) {
+       vectors := []struct {
+               input int64
+               width int
+               ok    bool
+       }{
+               {-1, 1, false},
+               {-1, 2, false},
+               {-1, 3, false},
+               {0, 1, true},
+               {0 + 1, 1, false},
+               {0, 2, true},
+               {07, 2, true},
+               {07 + 1, 2, false},
+               {0, 4, true},
+               {0777, 4, true},
+               {0777 + 1, 4, false},
+               {0, 8, true},
+               {07777777, 8, true},
+               {07777777 + 1, 8, false},
+               {0, 12, true},
+               {077777777777, 12, true},
+               {077777777777 + 1, 12, false},
+               {math.MaxInt64, 22, true},
+               {012345670123, 12, true},
+               {01564164, 12, true},
+               {-012345670123, 12, false},
+               {-01564164, 12, false},
+               {-1564164, 30, false},
+       }
+
+       for _, v := range vectors {
+               ok := fitsInOctal(v.width, v.input)
+               if ok != v.ok {
+                       t.Errorf("checkOctal(%d, %d): got %v, want %v", v.input, v.width, ok, v.ok)
+               }
+       }
+}
+
 func TestParsePAXTime(t *testing.T) {
        vectors := []struct {
                in   string

diff --git a/src/archive/tar/writer.go b/src/archive/tar/writer.go
index 8d06e1145cc6dffae8692ffb9d32bec41db9db8b..2fdf53f59815c6142eec43db613f9778f416fcb2 100644 (file)
--- a/src/archive/tar/writer.go
+++ b/src/archive/tar/writer.go
@@ -132,22 +132,17 @@ func (tw *Writer) writeHeader(hdr *Header, allowPax bool) error {
                f.formatString(b, s) // Should never error
        }
        var formatNumeric = func(b []byte, x int64, paxKeyword string) {
-               // Try octal first.
-               s := strconv.FormatInt(x, 8)
-               if len(s) < len(b) {
-                       f.formatOctal(b, x)
-                       return
+               if !fitsInOctal(len(b), x) {
+                       if paxKeyword != paxNone && tw.preferPax {
+                               // Use PAX format.
+                               f.formatOctal(b, 0)
+                               paxHeaders[paxKeyword] = strconv.FormatInt(x, 10)
+                               return
+                       } else {
+                               // Use GNU format.
+                               tw.usedBinary = true
+                       }
                }
-
-               // If it is too long for octal, and PAX is preferred, use a PAX header.
-               if paxKeyword != paxNone && tw.preferPax {
-                       f.formatOctal(b, 0)
-                       s := strconv.FormatInt(x, 10)
-                       paxHeaders[paxKeyword] = s
-                       return
-               }
-
-               tw.usedBinary = true
                f.formatNumeric(b, x)
        }