aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRoland Shoemaker <roland@golang.org>2022-03-29 15:52:09 -0700
committerMichael Knyszek <mknyszek@google.com>2022-07-12 15:05:33 +0000
commitc4c1993fd2a5b26fe45c09592af6d3388a3b2e08 (patch)
treec9fcd79b62f65e7c2bda9d3e4bb6e4a34a7928a5
parent913d05133c7fb3adfd2b1a34a47d635d8e072fa2 (diff)
downloadgo-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.go27
-rw-r--r--src/encoding/xml/read_test.go15
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)
+ }
+}