archive/tar: reject bad key-value pairs for PAX records

We forbid empty keys or keys with '=' because it leads to ambiguous parsing.
Relevent PAX specification:
<<<
A keyword shall not include an <equals-sign>.

>
> Also, we forbid the writer from encoding records with an empty value.
> While, this is a valid record syntactically, the semantics of an empty
> value is that previous records with that key should be deleted.
> Since we have no support (and probably never will) for global PAX records,
> deletion is a non-sensible operation.
> <<<
> If the <value> field is zero length,
> it shall delete any header block field,
> previously entered extended header value,
> or global extended header value of the same name.


Fixes #20698
Fixes #15567

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

src/archive/tar/common.go

@@ -160,7 +160,9 @@ func (h *Header) allowedFormats() (format int, paxHdrs map[string]string) {
 		format &= formatPAX // PAX only
 	}
 	for k, v := range paxHdrs {
-		if !validPAXRecord(k, v) {
+		// Forbid empty values (which represent deletion) since usage of
+		// them are non-sensible without global PAX record support.
+		if !validPAXRecord(k, v) || v == "" {
 			return formatUnknown, nil // Invalid PAX key
 		}
 	}

src/archive/tar/strconv.go

@@ -240,9 +240,6 @@ func formatPAXTime(ts time.Time) (s string) {
 // parsePAXRecord parses the input PAX record string into a key-value pair.
 // If parsing is successful, it will slice off the currently read record and
 // return the remainder as r.
-//
-// A PAX record is of the following form:
-//	"%d %s=%s\n" % (size, key, value)
 func parsePAXRecord(s string) (k, v, r string, err error) {
 	// The size field ends at the first space.
 	sp := strings.IndexByte(s, ' ')
@@ -295,12 +292,19 @@ func formatPAXRecord(k, v string) (string, error) {
 	return record, nil
 }
 
-// validPAXRecord reports whether the key-value pair is valid.
+// validPAXRecord reports whether the key-value pair is valid where each
+// record is formatted as:
+//	"%d %s=%s\n" % (size, key, value)
+//
 // Keys and values should be UTF-8, but the number of bad writers out there
 // forces us to be a more liberal.
 // Thus, we only reject all keys with NUL, and only reject NULs in values
 // for the PAX version of the USTAR string fields.
+// The key must not contain an '=' character.
 func validPAXRecord(k, v string) bool {
+	if k == "" || strings.IndexByte(k, '=') >= 0 {
+		return false
+	}
 	switch k {
 	case paxPath, paxLinkpath, paxUname, paxGname:
 		return strings.IndexByte(v, 0) < 0

src/archive/tar/tar_test.go

@@ -438,6 +438,12 @@ func TestHeaderAllowedFormats(t *testing.T) {
 		header:  &Header{Xattrs: map[string]string{"用戶名": "\x00hello"}},
 		paxHdrs: map[string]string{paxXattr + "用戶名": "\x00hello"},
 		formats: formatPAX,
+	}, {
+		header:  &Header{Xattrs: map[string]string{"foo=bar": "baz"}},
+		formats: formatUnknown,
+	}, {
+		header:  &Header{Xattrs: map[string]string{"foo": ""}},
+		formats: formatUnknown,
 	}, {
 		header:  &Header{ModTime: time.Unix(0, 0)},
 		formats: formatUSTAR | formatPAX | formatGNU,