diff options
author | Roland Shoemaker <roland@golang.org> | 2022-03-29 15:52:09 -0700 |
---|---|---|
committer | Michael Knyszek <mknyszek@google.com> | 2022-07-12 15:05:33 +0000 |
commit | c4c1993fd2a5b26fe45c09592af6d3388a3b2e08 (patch) | |
tree | c9fcd79b62f65e7c2bda9d3e4bb6e4a34a7928a5 | |
parent | 913d05133c7fb3adfd2b1a34a47d635d8e072fa2 (diff) | |
download | go-c4c1993fd2a5b26fe45c09592af6d3388a3b2e08.tar.gz go-c4c1993fd2a5b26fe45c09592af6d3388a3b2e08.zip |
encoding/xml: limit depth of nesting in unmarshal
Prevent exhausting the stack limit when unmarshalling extremely deeply
nested structures into nested types.
Fixes #53611
Fixes CVE-2022-30633
Change-Id: Ic6c5d41674c93cfc9a316135a408db9156d39c59
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1421319
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/417061
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
-rw-r--r-- | src/encoding/xml/read.go | 27 | ||||
-rw-r--r-- | src/encoding/xml/read_test.go | 15 |
2 files changed, 34 insertions, 8 deletions
diff --git a/src/encoding/xml/read.go b/src/encoding/xml/read.go index 257591262f..01613065e3 100644 --- a/src/encoding/xml/read.go +++ b/src/encoding/xml/read.go @@ -152,7 +152,7 @@ func (d *Decoder) DecodeElement(v any, start *StartElement) error { if val.IsNil() { return errors.New("nil pointer passed to Unmarshal") } - return d.unmarshal(val.Elem(), start) + return d.unmarshal(val.Elem(), start, 0) } // An UnmarshalError represents an error in the unmarshaling process. @@ -308,8 +308,15 @@ var ( textUnmarshalerType = reflect.TypeOf((*encoding.TextUnmarshaler)(nil)).Elem() ) +const maxUnmarshalDepth = 10000 + +var errExeceededMaxUnmarshalDepth = errors.New("exceeded max depth") + // Unmarshal a single XML element into val. -func (d *Decoder) unmarshal(val reflect.Value, start *StartElement) error { +func (d *Decoder) unmarshal(val reflect.Value, start *StartElement, depth int) error { + if depth >= maxUnmarshalDepth { + return errExeceededMaxUnmarshalDepth + } // Find start element if we need it. if start == nil { for { @@ -402,7 +409,7 @@ func (d *Decoder) unmarshal(val reflect.Value, start *StartElement) error { v.Set(reflect.Append(val, reflect.Zero(v.Type().Elem()))) // Recur to read element into slice. - if err := d.unmarshal(v.Index(n), start); err != nil { + if err := d.unmarshal(v.Index(n), start, depth+1); err != nil { v.SetLen(n) return err } @@ -525,13 +532,15 @@ Loop: case StartElement: consumed := false if sv.IsValid() { - consumed, err = d.unmarshalPath(tinfo, sv, nil, &t) + // unmarshalPath can call unmarshal, so we need to pass the depth through so that + // we can continue to enforce the maximum recusion limit. + consumed, err = d.unmarshalPath(tinfo, sv, nil, &t, depth) if err != nil { return err } if !consumed && saveAny.IsValid() { consumed = true - if err := d.unmarshal(saveAny, &t); err != nil { + if err := d.unmarshal(saveAny, &t, depth+1); err != nil { return err } } @@ -676,7 +685,7 @@ func copyValue(dst reflect.Value, src []byte) (err error) { // The consumed result tells whether XML elements have been consumed // from the Decoder until start's matching end element, or if it's // still untouched because start is uninteresting for sv's fields. -func (d *Decoder) unmarshalPath(tinfo *typeInfo, sv reflect.Value, parents []string, start *StartElement) (consumed bool, err error) { +func (d *Decoder) unmarshalPath(tinfo *typeInfo, sv reflect.Value, parents []string, start *StartElement, depth int) (consumed bool, err error) { recurse := false Loop: for i := range tinfo.fields { @@ -691,7 +700,7 @@ Loop: } if len(finfo.parents) == len(parents) && finfo.name == start.Name.Local { // It's a perfect match, unmarshal the field. - return true, d.unmarshal(finfo.value(sv, initNilPointers), start) + return true, d.unmarshal(finfo.value(sv, initNilPointers), start, depth+1) } if len(finfo.parents) > len(parents) && finfo.parents[len(parents)] == start.Name.Local { // It's a prefix for the field. Break and recurse @@ -720,7 +729,9 @@ Loop: } switch t := tok.(type) { case StartElement: - consumed2, err := d.unmarshalPath(tinfo, sv, parents, &t) + // the recursion depth of unmarshalPath is limited to the path length specified + // by the struct field tag, so we don't increment the depth here. + consumed2, err := d.unmarshalPath(tinfo, sv, parents, &t, depth) if err != nil { return true, err } diff --git a/src/encoding/xml/read_test.go b/src/encoding/xml/read_test.go index 6ef55de77b..42059db3ae 100644 --- a/src/encoding/xml/read_test.go +++ b/src/encoding/xml/read_test.go @@ -5,6 +5,8 @@ package xml import ( + "bytes" + "errors" "io" "reflect" "strings" @@ -1094,3 +1096,16 @@ func TestUnmarshalIntoNil(t *testing.T) { } } + +func TestCVE202228131(t *testing.T) { + type nested struct { + Parent *nested `xml:",any"` + } + var n nested + err := Unmarshal(bytes.Repeat([]byte("<a>"), maxUnmarshalDepth+1), &n) + if err == nil { + t.Fatal("Unmarshal did not fail") + } else if !errors.Is(err, errExeceededMaxUnmarshalDepth) { + t.Fatalf("Unmarshal unexpected error: got %q, want %q", err, errExeceededMaxUnmarshalDepth) + } +} |