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>

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.

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

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.
+		if !fitsInOctal(len(b), x) {
-		s := strconv.FormatInt(x, 8)
+			if paxKeyword != paxNone && tw.preferPax {
-		if len(s) < len(b) {
+				// Use PAX format.
-			f.formatOctal(b, x)
+				f.formatOctal(b, 0)
-			return
+				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)
 	}