From 1da0e7e28ed3694d2a50b051ce556d06b90789ef Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 14 Aug 2017 15:57:46 -0700 Subject: [PATCH] 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 . >>> 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 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/55571 Reviewed-by: Ian Lance Taylor --- src/archive/tar/common.go | 4 +++- src/archive/tar/strconv.go | 12 ++++++++---- src/archive/tar/tar_test.go | 6 ++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/archive/tar/common.go b/src/archive/tar/common.go index 5b921486f1..e9dff8439e 100644 --- a/src/archive/tar/common.go +++ b/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 } } diff --git a/src/archive/tar/strconv.go b/src/archive/tar/strconv.go index a93fc4ac7a..89ac8112e5 100644 --- a/src/archive/tar/strconv.go +++ b/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 diff --git a/src/archive/tar/tar_test.go b/src/archive/tar/tar_test.go index e1d64a6957..79895e6f9d 100644 --- a/src/archive/tar/tar_test.go +++ b/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,