|
| 1 | +From 2092294f2b097c5828f4eace6c98a322c1510b01 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Roland Shoemaker < [email protected]> |
| 3 | +Date: Fri, 3 May 2024 09:21:39 -0400 |
| 4 | +Subject: [PATCH] [release-branch.go1.22] encoding/gob: cover missed cases when |
| 5 | + checking ignore depth |
| 6 | + |
| 7 | +This change makes sure that we are properly checking the ignored field |
| 8 | +recursion depth in decIgnoreOpFor consistently. This prevents stack |
| 9 | +exhaustion when attempting to decode a message that contains an |
| 10 | +extremely deeply nested struct which is ignored. |
| 11 | + |
| 12 | +Thanks to Md Sakib Anwar of The Ohio State University ( [email protected]) |
| 13 | +for reporting this issue. |
| 14 | + |
| 15 | +Updates #69139 |
| 16 | +Fixes #69144 |
| 17 | +Fixes CVE-2024-34156 |
| 18 | + |
| 19 | +Change-Id: Iacce06be95a5892b3064f1c40fcba2e2567862d6 |
| 20 | +Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1440 |
| 21 | +Reviewed-by: Russ Cox < [email protected]> |
| 22 | +Reviewed-by: Damien Neil < [email protected]> |
| 23 | +(cherry picked from commit f0a11f9b3aaa362cb1d05e095e3c8d421d4f087f) |
| 24 | +Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1580 |
| 25 | +Reviewed-by: Tatiana Bradley < [email protected]> |
| 26 | +Reviewed-on: https://go-review.googlesource.com/c/go/+/611182 |
| 27 | +TryBot-Bypass: Dmitri Shuralyov < [email protected]> |
| 28 | +Reviewed-by: Michael Pratt < [email protected]> |
| 29 | +Auto-Submit: Dmitri Shuralyov < [email protected]> |
| 30 | +Reviewed-by: Dmitri Shuralyov < [email protected]> |
| 31 | +--- |
| 32 | + src/encoding/gob/decode.go | 19 +++++++++++-------- |
| 33 | + src/encoding/gob/decoder.go | 2 ++ |
| 34 | + src/encoding/gob/gobencdec_test.go | 14 ++++++++++++++ |
| 35 | + 3 files changed, 27 insertions(+), 8 deletions(-) |
| 36 | + |
| 37 | +Backported by the golang-fips authors. |
| 38 | + |
| 39 | +diff --git a/src/encoding/gob/decode.go b/src/encoding/gob/decode.go |
| 40 | +index c0b054ef80..a2d4eabe46 100644 |
| 41 | +--- a/src/encoding/gob/decode.go |
| 42 | ++++ b/src/encoding/gob/decode.go |
| 43 | +@@ -911,8 +911,11 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg |
| 44 | + var maxIgnoreNestingDepth = 10000 |
| 45 | + |
| 46 | + // decIgnoreOpFor returns the decoding op for a field that has no destination. |
| 47 | +-func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp, depth int) *decOp { |
| 48 | +- if depth > maxIgnoreNestingDepth { |
| 49 | ++func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp) *decOp { |
| 50 | ++ // Track how deep we've recursed trying to skip nested ignored fields. |
| 51 | ++ dec.ignoreDepth++ |
| 52 | ++ defer func() { dec.ignoreDepth-- }() |
| 53 | ++ if dec.ignoreDepth > maxIgnoreNestingDepth { |
| 54 | + error_(errors.New("invalid nesting depth")) |
| 55 | + } |
| 56 | + // If this type is already in progress, it's a recursive type (e.g. map[string]*T). |
| 57 | +@@ -938,7 +941,7 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp, |
| 58 | + errorf("bad data: undefined type %s", wireId.string()) |
| 59 | + case wire.ArrayT != nil: |
| 60 | + elemId := wire.ArrayT.Elem |
| 61 | +- elemOp := dec.decIgnoreOpFor(elemId, inProgress, depth+1) |
| 62 | ++ elemOp := dec.decIgnoreOpFor(elemId, inProgress) |
| 63 | + op = func(i *decInstr, state *decoderState, value reflect.Value) { |
| 64 | + state.dec.ignoreArray(state, *elemOp, wire.ArrayT.Len) |
| 65 | + } |
| 66 | +@@ -946,15 +949,15 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp, |
| 67 | + case wire.MapT != nil: |
| 68 | + keyId := dec.wireType[wireId].MapT.Key |
| 69 | + elemId := dec.wireType[wireId].MapT.Elem |
| 70 | +- keyOp := dec.decIgnoreOpFor(keyId, inProgress, depth+1) |
| 71 | +- elemOp := dec.decIgnoreOpFor(elemId, inProgress, depth+1) |
| 72 | ++ keyOp := dec.decIgnoreOpFor(keyId, inProgress) |
| 73 | ++ elemOp := dec.decIgnoreOpFor(elemId, inProgress) |
| 74 | + op = func(i *decInstr, state *decoderState, value reflect.Value) { |
| 75 | + state.dec.ignoreMap(state, *keyOp, *elemOp) |
| 76 | + } |
| 77 | + |
| 78 | + case wire.SliceT != nil: |
| 79 | + elemId := wire.SliceT.Elem |
| 80 | +- elemOp := dec.decIgnoreOpFor(elemId, inProgress, depth+1) |
| 81 | ++ elemOp := dec.decIgnoreOpFor(elemId, inProgress) |
| 82 | + op = func(i *decInstr, state *decoderState, value reflect.Value) { |
| 83 | + state.dec.ignoreSlice(state, *elemOp) |
| 84 | + } |
| 85 | +@@ -1115,7 +1118,7 @@ func (dec *Decoder) compileSingle(remoteId typeId, ut *userTypeInfo) (engine *de |
| 86 | + func (dec *Decoder) compileIgnoreSingle(remoteId typeId) *decEngine { |
| 87 | + engine := new(decEngine) |
| 88 | + engine.instr = make([]decInstr, 1) // one item |
| 89 | +- op := dec.decIgnoreOpFor(remoteId, make(map[typeId]*decOp), 0) |
| 90 | ++ op := dec.decIgnoreOpFor(remoteId, make(map[typeId]*decOp)) |
| 91 | + ovfl := overflow(dec.typeString(remoteId)) |
| 92 | + engine.instr[0] = decInstr{*op, 0, nil, ovfl} |
| 93 | + engine.numInstr = 1 |
| 94 | +@@ -1160,7 +1163,7 @@ func (dec *Decoder) compileDec(remoteId typeId, ut *userTypeInfo) (engine *decEn |
| 95 | + localField, present := srt.FieldByName(wireField.Name) |
| 96 | + // TODO(r): anonymous names |
| 97 | + if !present || !isExported(wireField.Name) { |
| 98 | +- op := dec.decIgnoreOpFor(wireField.Id, make(map[typeId]*decOp), 0) |
| 99 | ++ op := dec.decIgnoreOpFor(wireField.Id, make(map[typeId]*decOp)) |
| 100 | + engine.instr[fieldnum] = decInstr{*op, fieldnum, nil, ovfl} |
| 101 | + continue |
| 102 | + } |
| 103 | +diff --git a/src/encoding/gob/decoder.go b/src/encoding/gob/decoder.go |
| 104 | +index 5b77adc7e8..4da5717092 100644 |
| 105 | +--- a/src/encoding/gob/decoder.go |
| 106 | ++++ b/src/encoding/gob/decoder.go |
| 107 | +@@ -35,6 +35,8 @@ type Decoder struct { |
| 108 | + freeList *decoderState // list of free decoderStates; avoids reallocation |
| 109 | + countBuf []byte // used for decoding integers while parsing messages |
| 110 | + err error |
| 111 | ++ // ignoreDepth tracks the depth of recursively parsed ignored fields |
| 112 | ++ ignoreDepth int |
| 113 | + } |
| 114 | + |
| 115 | + // NewDecoder returns a new decoder that reads from the io.Reader. |
| 116 | +diff --git a/src/encoding/gob/gobencdec_test.go b/src/encoding/gob/gobencdec_test.go |
| 117 | +index 6fefd36756..3955e281ae 100644 |
| 118 | +--- a/src/encoding/gob/gobencdec_test.go |
| 119 | ++++ b/src/encoding/gob/gobencdec_test.go |
| 120 | +@@ -806,6 +806,8 @@ func TestIgnoreDepthLimit(t *testing.T) { |
| 121 | + defer func() { maxIgnoreNestingDepth = oldNestingDepth }() |
| 122 | + b := new(bytes.Buffer) |
| 123 | + enc := NewEncoder(b) |
| 124 | ++ |
| 125 | ++ // Nested slice |
| 126 | + typ := reflect.TypeOf(int(0)) |
| 127 | + nested := reflect.ArrayOf(1, typ) |
| 128 | + for i := 0; i < 100; i++ { |
| 129 | +@@ -819,4 +821,16 @@ func TestIgnoreDepthLimit(t *testing.T) { |
| 130 | + if err := dec.Decode(&output); err == nil || err.Error() != expectedErr { |
| 131 | + t.Errorf("Decode didn't fail with depth limit of 100: want %q, got %q", expectedErr, err) |
| 132 | + } |
| 133 | ++ |
| 134 | ++ // Nested struct |
| 135 | ++ nested = reflect.StructOf([]reflect.StructField{{Name: "F", Type: typ}}) |
| 136 | ++ for i := 0; i < 100; i++ { |
| 137 | ++ nested = reflect.StructOf([]reflect.StructField{{Name: "F", Type: nested}}) |
| 138 | ++ } |
| 139 | ++ badStruct = reflect.New(reflect.StructOf([]reflect.StructField{{Name: "F", Type: nested}})) |
| 140 | ++ enc.Encode(badStruct.Interface()) |
| 141 | ++ dec = NewDecoder(b) |
| 142 | ++ if err := dec.Decode(&output); err == nil || err.Error() != expectedErr { |
| 143 | ++ t.Errorf("Decode didn't fail with depth limit of 100: want %q, got %q", expectedErr, err) |
| 144 | ++ } |
| 145 | + } |
0 commit comments