From 032ac075c20c01c6c35a672d1542d3e98eab84ea Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Thu, 20 Jun 2024 10:45:30 -0700 Subject: [PATCH] [release-branch.go1.23] go/build/constraint: add parsing limits Limit the size of build constraints that we will parse. This prevents a number of stack exhaustions that can be hit when parsing overly complex constraints. The imposed limits are unlikely to ever be hit in real world usage. Updates #69141 Fixes #69149 Fixes CVE-2024-34158 Change-Id: I38b614bf04caa36eefc6a4350d848588c4cef3c4 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1540 Reviewed-by: Damien Neil Reviewed-by: Russ Cox (cherry picked from commit 0c74dc9e0da0cf1e12494b514d822b5bebbc9f04) Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1562 Commit-Queue: Roland Shoemaker Reviewed-by: Tatiana Bradley Reviewed-on: https://go-review.googlesource.com/c/go/+/611177 Reviewed-by: Michael Pratt TryBot-Bypass: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov Auto-Submit: Dmitri Shuralyov --- src/go/build/constraint/expr.go | 28 ++++++++++-- src/go/build/constraint/expr_test.go | 65 +++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/src/go/build/constraint/expr.go b/src/go/build/constraint/expr.go index e59012361b..0f05f8db6a 100644 --- a/src/go/build/constraint/expr.go +++ b/src/go/build/constraint/expr.go @@ -16,6 +16,10 @@ import ( "unicode/utf8" ) +// maxSize is a limit used to control the complexity of expressions, in order +// to prevent stack exhaustion issues due to recursion. +const maxSize = 1000 + // An Expr is a build tag constraint expression. // The underlying concrete type is *[AndExpr], *[OrExpr], *[NotExpr], or *[TagExpr]. type Expr interface { @@ -151,7 +155,7 @@ func Parse(line string) (Expr, error) { return parseExpr(text) } if text, ok := splitPlusBuild(line); ok { - return parsePlusBuildExpr(text), nil + return parsePlusBuildExpr(text) } return nil, errNotConstraint } @@ -201,6 +205,8 @@ type exprParser struct { tok string // last token read isTag bool pos int // position (start) of last token + + size int } // parseExpr parses a boolean build tag expression. @@ -249,6 +255,10 @@ func (p *exprParser) and() Expr { // On entry, the next input token has not yet been lexed. // On exit, the next input token has been lexed and is in p.tok. func (p *exprParser) not() Expr { + p.size++ + if p.size > maxSize { + panic(&SyntaxError{Offset: p.pos, Err: "build expression too large"}) + } p.lex() if p.tok == "!" { p.lex() @@ -388,7 +398,13 @@ func splitPlusBuild(line string) (expr string, ok bool) { } // parsePlusBuildExpr parses a legacy build tag expression (as used with “// +build”). -func parsePlusBuildExpr(text string) Expr { +func parsePlusBuildExpr(text string) (Expr, error) { + // Only allow up to 100 AND/OR operators for "old" syntax. + // This is much less than the limit for "new" syntax, + // but uses of old syntax were always very simple. + const maxOldSize = 100 + size := 0 + var x Expr for _, clause := range strings.Fields(text) { var y Expr @@ -414,19 +430,25 @@ func parsePlusBuildExpr(text string) Expr { if y == nil { y = z } else { + if size++; size > maxOldSize { + return nil, errComplex + } y = and(y, z) } } if x == nil { x = y } else { + if size++; size > maxOldSize { + return nil, errComplex + } x = or(x, y) } } if x == nil { x = tag("ignore") } - return x + return x, nil } // isValidTag reports whether the word is a valid build tag. diff --git a/src/go/build/constraint/expr_test.go b/src/go/build/constraint/expr_test.go index 15d189012e..ac38ba6929 100644 --- a/src/go/build/constraint/expr_test.go +++ b/src/go/build/constraint/expr_test.go @@ -222,7 +222,7 @@ var parsePlusBuildExprTests = []struct { func TestParsePlusBuildExpr(t *testing.T) { for i, tt := range parsePlusBuildExprTests { t.Run(fmt.Sprint(i), func(t *testing.T) { - x := parsePlusBuildExpr(tt.in) + x, _ := parsePlusBuildExpr(tt.in) if x.String() != tt.x.String() { t.Errorf("parsePlusBuildExpr(%q):\nhave %v\nwant %v", tt.in, x, tt.x) } @@ -319,3 +319,66 @@ func TestPlusBuildLines(t *testing.T) { }) } } + +func TestSizeLimits(t *testing.T) { + for _, tc := range []struct { + name string + expr string + }{ + { + name: "go:build or limit", + expr: "//go:build " + strings.Repeat("a || ", maxSize+2), + }, + { + name: "go:build and limit", + expr: "//go:build " + strings.Repeat("a && ", maxSize+2), + }, + { + name: "go:build and depth limit", + expr: "//go:build " + strings.Repeat("(a &&", maxSize+2), + }, + { + name: "go:build or depth limit", + expr: "//go:build " + strings.Repeat("(a ||", maxSize+2), + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := Parse(tc.expr) + if err == nil { + t.Error("expression did not trigger limit") + } else if syntaxErr, ok := err.(*SyntaxError); !ok || syntaxErr.Err != "build expression too large" { + if !ok { + t.Errorf("unexpected error: %v", err) + } else { + t.Errorf("unexpected syntax error: %s", syntaxErr.Err) + } + } + }) + } +} + +func TestPlusSizeLimits(t *testing.T) { + maxOldSize := 100 + for _, tc := range []struct { + name string + expr string + }{ + { + name: "+build or limit", + expr: "// +build " + strings.Repeat("a ", maxOldSize+2), + }, + { + name: "+build and limit", + expr: "// +build " + strings.Repeat("a,", maxOldSize+2), + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := Parse(tc.expr) + if err == nil { + t.Error("expression did not trigger limit") + } else if err != errComplex { + t.Errorf("unexpected error: got %q, want %q", err, errComplex) + } + }) + } +} -- 2.48.1