encoding/xml: handle leading, trailing, or double colons in names

Before this change, <:name> would parse as <name>, which could cause
issues in applications that rely on the parse-encode cycle to
round-trip. Similarly, <x name:=""> would parse as expected but then
have the attribute dropped when serializing because its name was empty.
Finally, <a:b:c> would parse and get serialized incorrectly. All these
values are invalid XML, but to minimize the impact of this change, we
parse them whole into Name.Local.

This issue was reported by Juho Nurminen of Mattermost as it leads to
round-trip mismatches. See #43168. It's not being fixed in a security
release because round-trip stability is not a currently supported
security property of encoding/xml, and we don't believe these fixes
would be sufficient to reliably guarantee it in the future.

Fixes CVE-2020-29509
Fixes CVE-2020-29511
Updates #43168

Change-Id: I68321c4d867305046f664347192948a889af3c7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/277892
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>

2 files changed, 59 insertions, 2 deletions

diff --git a/src/encoding/xml/xml.go b/src/encoding/xml/xml.go
index 384d6ad4b8..c902f1295a 100644
--- a/src/encoding/xml/xml.go
+++ b/src/encoding/xml/xml.go
@@ -1156,8 +1156,9 @@ func (d *Decoder) nsname() (name Name, ok bool) {
 	if !ok {
 		return
 	}
-	i := strings.Index(s, ":")
-	if i < 0 {
+	if strings.Count(s, ":") > 1 {
+		name.Local = s
+	} else if i := strings.Index(s, ":"); i < 1 || i > len(s)-2 {
 		name.Local = s
 	} else {
 		name.Space = s[0:i]
diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go
index 5a10f5309d..47d0c39167 100644
--- a/src/encoding/xml/xml_test.go
+++ b/src/encoding/xml/xml_test.go
@@ -1003,3 +1003,59 @@ func TestTokenUnmarshaler(t *testing.T) {
 	d := NewTokenDecoder(tokReader{})
 	d.Decode(&Failure{})
 }
+
+func testRoundTrip(t *testing.T, input string) {
+	d := NewDecoder(strings.NewReader(input))
+	var tokens []Token
+	var buf bytes.Buffer
+	e := NewEncoder(&buf)
+	for {
+		tok, err := d.Token()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatalf("invalid input: %v", err)
+		}
+		if err := e.EncodeToken(tok); err != nil {
+			t.Fatalf("failed to re-encode input: %v", err)
+		}
+		tokens = append(tokens, CopyToken(tok))
+	}
+	if err := e.Flush(); err != nil {
+		t.Fatal(err)
+	}
+
+	d = NewDecoder(&buf)
+	for {
+		tok, err := d.Token()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatalf("failed to decode output: %v", err)
+		}
+		if len(tokens) == 0 {
+			t.Fatalf("unexpected token: %#v", tok)
+		}
+		a, b := tokens[0], tok
+		if !reflect.DeepEqual(a, b) {
+			t.Fatalf("token mismatch: %#v vs %#v", a, b)
+		}
+		tokens = tokens[1:]
+	}
+	if len(tokens) > 0 {
+		t.Fatalf("lost tokens: %#v", tokens)
+	}
+}
+
+func TestRoundTrip(t *testing.T) {
+	tests := map[string]string{
+		"leading colon":  `<::Test ::foo="bar"><:::Hello></:::Hello><Hello></Hello></::Test>`,
+		"trailing colon": `<foo abc:="x"></foo>`,
+		"double colon":   `<x:y:foo></x:y:foo>`,
+	}
+	for name, input := range tests {
+		t.Run(name, func(t *testing.T) { testRoundTrip(t, input) })
+	}
+}