Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Go 1.19] Backport CVE fixes from Go 1.22.7 #232

Merged
merged 1 commit into from
Sep 12, 2024
Merged

[Go 1.19] Backport CVE fixes from Go 1.22.7 #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions patches/009-encoding-gob.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
From 2092294f2b097c5828f4eace6c98a322c1510b01 Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <[email protected]>
Date: Fri, 3 May 2024 09:21:39 -0400
Subject: [PATCH] [release-branch.go1.22] encoding/gob: cover missed cases when
checking ignore depth

This change makes sure that we are properly checking the ignored field
recursion depth in decIgnoreOpFor consistently. This prevents stack
exhaustion when attempting to decode a message that contains an
extremely deeply nested struct which is ignored.

Thanks to Md Sakib Anwar of The Ohio State University ([email protected])
for reporting this issue.

Updates #69139
Fixes #69144
Fixes CVE-2024-34156

Change-Id: Iacce06be95a5892b3064f1c40fcba2e2567862d6
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1440
Reviewed-by: Russ Cox <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
(cherry picked from commit f0a11f9b3aaa362cb1d05e095e3c8d421d4f087f)
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1580
Reviewed-by: Tatiana Bradley <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/611182
TryBot-Bypass: Dmitri Shuralyov <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
---
src/encoding/gob/decode.go | 19 +++++++++++--------
src/encoding/gob/decoder.go | 2 ++
src/encoding/gob/gobencdec_test.go | 14 ++++++++++++++
3 files changed, 27 insertions(+), 8 deletions(-)

Backported by the golang-fips authors.

diff --git a/src/encoding/gob/decode.go b/src/encoding/gob/decode.go
index c0b054ef80..a2d4eabe46 100644
--- a/src/encoding/gob/decode.go
+++ b/src/encoding/gob/decode.go
@@ -911,8 +911,11 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg
var maxIgnoreNestingDepth = 10000

// decIgnoreOpFor returns the decoding op for a field that has no destination.
-func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp, depth int) *decOp {
- if depth > maxIgnoreNestingDepth {
+func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp) *decOp {
+ // Track how deep we've recursed trying to skip nested ignored fields.
+ dec.ignoreDepth++
+ defer func() { dec.ignoreDepth-- }()
+ if dec.ignoreDepth > maxIgnoreNestingDepth {
error_(errors.New("invalid nesting depth"))
}
// If this type is already in progress, it's a recursive type (e.g. map[string]*T).
@@ -938,7 +941,7 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp,
errorf("bad data: undefined type %s", wireId.string())
case wire.ArrayT != nil:
elemId := wire.ArrayT.Elem
- elemOp := dec.decIgnoreOpFor(elemId, inProgress, depth+1)
+ elemOp := dec.decIgnoreOpFor(elemId, inProgress)
op = func(i *decInstr, state *decoderState, value reflect.Value) {
state.dec.ignoreArray(state, *elemOp, wire.ArrayT.Len)
}
@@ -946,15 +949,15 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId, inProgress map[typeId]*decOp,
case wire.MapT != nil:
keyId := dec.wireType[wireId].MapT.Key
elemId := dec.wireType[wireId].MapT.Elem
- keyOp := dec.decIgnoreOpFor(keyId, inProgress, depth+1)
- elemOp := dec.decIgnoreOpFor(elemId, inProgress, depth+1)
+ keyOp := dec.decIgnoreOpFor(keyId, inProgress)
+ elemOp := dec.decIgnoreOpFor(elemId, inProgress)
op = func(i *decInstr, state *decoderState, value reflect.Value) {
state.dec.ignoreMap(state, *keyOp, *elemOp)
}

case wire.SliceT != nil:
elemId := wire.SliceT.Elem
- elemOp := dec.decIgnoreOpFor(elemId, inProgress, depth+1)
+ elemOp := dec.decIgnoreOpFor(elemId, inProgress)
op = func(i *decInstr, state *decoderState, value reflect.Value) {
state.dec.ignoreSlice(state, *elemOp)
}
@@ -1115,7 +1118,7 @@ func (dec *Decoder) compileSingle(remoteId typeId, ut *userTypeInfo) (engine *de
func (dec *Decoder) compileIgnoreSingle(remoteId typeId) *decEngine {
engine := new(decEngine)
engine.instr = make([]decInstr, 1) // one item
- op := dec.decIgnoreOpFor(remoteId, make(map[typeId]*decOp), 0)
+ op := dec.decIgnoreOpFor(remoteId, make(map[typeId]*decOp))
ovfl := overflow(dec.typeString(remoteId))
engine.instr[0] = decInstr{*op, 0, nil, ovfl}
engine.numInstr = 1
@@ -1160,7 +1163,7 @@ func (dec *Decoder) compileDec(remoteId typeId, ut *userTypeInfo) (engine *decEn
localField, present := srt.FieldByName(wireField.Name)
// TODO(r): anonymous names
if !present || !isExported(wireField.Name) {
- op := dec.decIgnoreOpFor(wireField.Id, make(map[typeId]*decOp), 0)
+ op := dec.decIgnoreOpFor(wireField.Id, make(map[typeId]*decOp))
engine.instr[fieldnum] = decInstr{*op, fieldnum, nil, ovfl}
continue
}
diff --git a/src/encoding/gob/decoder.go b/src/encoding/gob/decoder.go
index 5b77adc7e8..4da5717092 100644
--- a/src/encoding/gob/decoder.go
+++ b/src/encoding/gob/decoder.go
@@ -35,6 +35,8 @@ type Decoder struct {
freeList *decoderState // list of free decoderStates; avoids reallocation
countBuf []byte // used for decoding integers while parsing messages
err error
+ // ignoreDepth tracks the depth of recursively parsed ignored fields
+ ignoreDepth int
}

// NewDecoder returns a new decoder that reads from the io.Reader.
diff --git a/src/encoding/gob/gobencdec_test.go b/src/encoding/gob/gobencdec_test.go
index 6fefd36756..3955e281ae 100644
--- a/src/encoding/gob/gobencdec_test.go
+++ b/src/encoding/gob/gobencdec_test.go
@@ -806,6 +806,8 @@ func TestIgnoreDepthLimit(t *testing.T) {
defer func() { maxIgnoreNestingDepth = oldNestingDepth }()
b := new(bytes.Buffer)
enc := NewEncoder(b)
+
+ // Nested slice
typ := reflect.TypeOf(int(0))
nested := reflect.ArrayOf(1, typ)
for i := 0; i < 100; i++ {
@@ -819,4 +821,16 @@ func TestIgnoreDepthLimit(t *testing.T) {
if err := dec.Decode(&output); err == nil || err.Error() != expectedErr {
t.Errorf("Decode didn't fail with depth limit of 100: want %q, got %q", expectedErr, err)
}
+
+ // Nested struct
+ nested = reflect.StructOf([]reflect.StructField{{Name: "F", Type: typ}})
+ for i := 0; i < 100; i++ {
+ nested = reflect.StructOf([]reflect.StructField{{Name: "F", Type: nested}})
+ }
+ badStruct = reflect.New(reflect.StructOf([]reflect.StructField{{Name: "F", Type: nested}}))
+ enc.Encode(badStruct.Interface())
+ dec = NewDecoder(b)
+ if err := dec.Decode(&output); err == nil || err.Error() != expectedErr {
+ t.Errorf("Decode didn't fail with depth limit of 100: want %q, got %q", expectedErr, err)
+ }
}
66 changes: 66 additions & 0 deletions patches/010-nested-elements.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
From b232596139dbe96a62edbe3a2a203e856bf556eb Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <[email protected]>
Date: Mon, 10 Jun 2024 15:34:12 -0700
Subject: [PATCH] [release-branch.go1.22] go/parser: track depth in nested
element lists

Prevents stack exhaustion with extremely deeply nested literal values,
i.e. field values in structs.

Updates #69138
Fixes #69142
Fixes CVE-2024-34155

Change-Id: I2e8e33b44105cc169d7ed1ae83fb56df0c10f1ee
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1520
Reviewed-by: Robert Griesemer <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
(cherry picked from commit eb1b038c0d01761694e7a735ef87ac9164c6568e)
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1561
Reviewed-by: Tatiana Bradley <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/611181
Reviewed-by: Michael Pratt <[email protected]>
TryBot-Bypass: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
---
src/go/parser/parser.go | 2 ++
src/go/parser/parser_test.go | 9 +++++----
2 files changed, 7 insertions(+), 4 deletions(-)

Backported by the golang-fips authors.

diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go
index 17808b366f092..f268dea1a6f9c 100644
--- a/src/go/parser/parser.go
+++ b/src/go/parser/parser.go
@@ -1676,6 +1676,8 @@ func (p *parser) parseElementList() (list []ast.Expr) {
}

func (p *parser) parseLiteralValue(typ ast.Expr) ast.Expr {
+ defer decNestLev(incNestLev(p))
+
if p.trace {
defer un(trace(p, "LiteralValue"))
}
diff --git a/src/go/parser/parser_test.go b/src/go/parser/parser_test.go
index 43b3416b27d7f..c6dca66760cca 100644
--- a/src/go/parser/parser_test.go
+++ b/src/go/parser/parser_test.go
@@ -598,10 +598,11 @@ var parseDepthTests = []struct {
{name: "chan2", format: "package main; var x «<-chan »int"},
{name: "interface", format: "package main; var x «interface { M() «int» }»", scope: true, scopeMultiplier: 2}, // Scopes: InterfaceType, FuncType
{name: "map", format: "package main; var x «map[int]»int"},
- {name: "slicelit", format: "package main; var x = «[]any{«»}»", parseMultiplier: 2}, // Parser nodes: UnaryExpr, CompositeLit
- {name: "arraylit", format: "package main; var x = «[1]any{«nil»}»", parseMultiplier: 2}, // Parser nodes: UnaryExpr, CompositeLit
- {name: "structlit", format: "package main; var x = «struct{x any}{«nil»}»", parseMultiplier: 2}, // Parser nodes: UnaryExpr, CompositeLit
- {name: "maplit", format: "package main; var x = «map[int]any{1:«nil»}»", parseMultiplier: 2}, // Parser nodes: CompositeLit, KeyValueExpr
+ {name: "slicelit", format: "package main; var x = []any{«[]any{«»}»}", parseMultiplier: 3}, // Parser nodes: UnaryExpr, CompositeLit
+ {name: "arraylit", format: "package main; var x = «[1]any{«nil»}»", parseMultiplier: 3}, // Parser nodes: UnaryExpr, CompositeLit
+ {name: "structlit", format: "package main; var x = «struct{x any}{«nil»}»", parseMultiplier: 3}, // Parser nodes: UnaryExpr, CompositeLit
+ {name: "maplit", format: "package main; var x = «map[int]any{1:«nil»}»", parseMultiplier: 3}, // Parser nodes: CompositeLit, KeyValueExpr
+ {name: "element", format: "package main; var x = struct{x any}{x: «{«»}»}"},
{name: "dot", format: "package main; var x = «x.»x"},
{name: "index", format: "package main; var x = x«[1]»"},
{name: "slice", format: "package main; var x = x«[1:2]»"},
Loading
Loading