| From 4d014e723165f28b34458edb4aa9136e0fb4c702 Mon Sep 17 00:00:00 2001 |
| From: Filippo Valsorda <filippo@golang.org> |
| Date: Tue, 27 Oct 2020 00:17:15 +0100 |
| Subject: [PATCH] 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> |
| |
| CVE: CVE-2020-29509 CVE-2020-29511 |
| Upstream-Status: Backport [4d014e723165f28b34458edb4aa9136e0fb4c702] |
| |
| Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com> |
| --- |
| src/encoding/xml/xml.go | 5 ++-- |
| src/encoding/xml/xml_test.go | 56 ++++++++++++++++++++++++++++++++++++ |
| 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) }) |
| + } |
| +} |
| -- |
| 2.25.1 |
| |