|
| 1 | +From d4c53812e6ce2ac368173d7fcd31d0ecfcffb002 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Roland Shoemaker <bracewell@google.com> |
| 3 | +Date: Thu, 20 Jun 2024 10:45:30 -0700 |
| 4 | +Subject: [PATCH] [release-branch.go1.22] go/build/constraint: add parsing |
| 5 | + limits |
| 6 | + |
| 7 | +Limit the size of build constraints that we will parse. This prevents a |
| 8 | +number of stack exhaustions that can be hit when parsing overly complex |
| 9 | +constraints. The imposed limits are unlikely to ever be hit in real |
| 10 | +world usage. |
| 11 | + |
| 12 | +Updates #69141 |
| 13 | +Fixes #69148 |
| 14 | +Fixes CVE-2024-34158 |
| 15 | + |
| 16 | +Change-Id: I38b614bf04caa36eefc6a4350d848588c4cef3c4 |
| 17 | +Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1540 |
| 18 | +Reviewed-by: Damien Neil <dneil@google.com> |
| 19 | +Reviewed-by: Russ Cox <rsc@google.com> |
| 20 | +(cherry picked from commit 0c74dc9e0da0cf1e12494b514d822b5bebbc9f04) |
| 21 | +Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1582 |
| 22 | +Reviewed-by: Tatiana Bradley <tatianabradley@google.com> |
| 23 | +Reviewed-on: https://go-review.googlesource.com/c/go/+/611183 |
| 24 | +Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> |
| 25 | +Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> |
| 26 | +Reviewed-by: Michael Pratt <mpratt@google.com> |
| 27 | +TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com> |
| 28 | +--- |
| 29 | + src/go/build/constraint/expr.go | 28 ++++++++++-- |
| 30 | + src/go/build/constraint/expr_test.go | 65 +++++++++++++++++++++++++++- |
| 31 | + 2 files changed, 89 insertions(+), 4 deletions(-) |
| 32 | + |
| 33 | +diff --git a/src/go/build/constraint/expr.go b/src/go/build/constraint/expr.go |
| 34 | +index e59012361bef6d..0f05f8db6a48cb 100644 |
| 35 | +--- a/src/go/build/constraint/expr.go |
| 36 | ++++ b/src/go/build/constraint/expr.go |
| 37 | +@@ -16,6 +16,10 @@ import ( |
| 38 | + "unicode/utf8" |
| 39 | + ) |
| 40 | + |
| 41 | ++// maxSize is a limit used to control the complexity of expressions, in order |
| 42 | ++// to prevent stack exhaustion issues due to recursion. |
| 43 | ++const maxSize = 1000 |
| 44 | ++ |
| 45 | + // An Expr is a build tag constraint expression. |
| 46 | + // The underlying concrete type is *[AndExpr], *[OrExpr], *[NotExpr], or *[TagExpr]. |
| 47 | + type Expr interface { |
| 48 | +@@ -151,7 +155,7 @@ func Parse(line string) (Expr, error) { |
| 49 | + return parseExpr(text) |
| 50 | + } |
| 51 | + if text, ok := splitPlusBuild(line); ok { |
| 52 | +- return parsePlusBuildExpr(text), nil |
| 53 | ++ return parsePlusBuildExpr(text) |
| 54 | + } |
| 55 | + return nil, errNotConstraint |
| 56 | + } |
| 57 | +@@ -201,6 +205,8 @@ type exprParser struct { |
| 58 | + tok string // last token read |
| 59 | + isTag bool |
| 60 | + pos int // position (start) of last token |
| 61 | ++ |
| 62 | ++ size int |
| 63 | + } |
| 64 | + |
| 65 | + // parseExpr parses a boolean build tag expression. |
| 66 | +@@ -249,6 +255,10 @@ func (p *exprParser) and() Expr { |
| 67 | + // On entry, the next input token has not yet been lexed. |
| 68 | + // On exit, the next input token has been lexed and is in p.tok. |
| 69 | + func (p *exprParser) not() Expr { |
| 70 | ++ p.size++ |
| 71 | ++ if p.size > maxSize { |
| 72 | ++ panic(&SyntaxError{Offset: p.pos, Err: "build expression too large"}) |
| 73 | ++ } |
| 74 | + p.lex() |
| 75 | + if p.tok == "!" { |
| 76 | + p.lex() |
| 77 | +@@ -388,7 +398,13 @@ func splitPlusBuild(line string) (expr string, ok bool) { |
| 78 | + } |
| 79 | + |
| 80 | + // parsePlusBuildExpr parses a legacy build tag expression (as used with “// +build”). |
| 81 | +-func parsePlusBuildExpr(text string) Expr { |
| 82 | ++func parsePlusBuildExpr(text string) (Expr, error) { |
| 83 | ++ // Only allow up to 100 AND/OR operators for "old" syntax. |
| 84 | ++ // This is much less than the limit for "new" syntax, |
| 85 | ++ // but uses of old syntax were always very simple. |
| 86 | ++ const maxOldSize = 100 |
| 87 | ++ size := 0 |
| 88 | ++ |
| 89 | + var x Expr |
| 90 | + for _, clause := range strings.Fields(text) { |
| 91 | + var y Expr |
| 92 | +@@ -414,19 +430,25 @@ func parsePlusBuildExpr(text string) Expr { |
| 93 | + if y == nil { |
| 94 | + y = z |
| 95 | + } else { |
| 96 | ++ if size++; size > maxOldSize { |
| 97 | ++ return nil, errComplex |
| 98 | ++ } |
| 99 | + y = and(y, z) |
| 100 | + } |
| 101 | + } |
| 102 | + if x == nil { |
| 103 | + x = y |
| 104 | + } else { |
| 105 | ++ if size++; size > maxOldSize { |
| 106 | ++ return nil, errComplex |
| 107 | ++ } |
| 108 | + x = or(x, y) |
| 109 | + } |
| 110 | + } |
| 111 | + if x == nil { |
| 112 | + x = tag("ignore") |
| 113 | + } |
| 114 | +- return x |
| 115 | ++ return x, nil |
| 116 | + } |
| 117 | + |
| 118 | + // isValidTag reports whether the word is a valid build tag. |
| 119 | +diff --git a/src/go/build/constraint/expr_test.go b/src/go/build/constraint/expr_test.go |
| 120 | +index 15d189012efb7d..ac38ba69294930 100644 |
| 121 | +--- a/src/go/build/constraint/expr_test.go |
| 122 | ++++ b/src/go/build/constraint/expr_test.go |
| 123 | +@@ -222,7 +222,7 @@ var parsePlusBuildExprTests = []struct { |
| 124 | + func TestParsePlusBuildExpr(t *testing.T) { |
| 125 | + for i, tt := range parsePlusBuildExprTests { |
| 126 | + t.Run(fmt.Sprint(i), func(t *testing.T) { |
| 127 | +- x := parsePlusBuildExpr(tt.in) |
| 128 | ++ x, _ := parsePlusBuildExpr(tt.in) |
| 129 | + if x.String() != tt.x.String() { |
| 130 | + t.Errorf("parsePlusBuildExpr(%q):\nhave %v\nwant %v", tt.in, x, tt.x) |
| 131 | + } |
| 132 | +@@ -319,3 +319,66 @@ func TestPlusBuildLines(t *testing.T) { |
| 133 | + }) |
| 134 | + } |
| 135 | + } |
| 136 | ++ |
| 137 | ++func TestSizeLimits(t *testing.T) { |
| 138 | ++ for _, tc := range []struct { |
| 139 | ++ name string |
| 140 | ++ expr string |
| 141 | ++ }{ |
| 142 | ++ { |
| 143 | ++ name: "go:build or limit", |
| 144 | ++ expr: "//go:build " + strings.Repeat("a || ", maxSize+2), |
| 145 | ++ }, |
| 146 | ++ { |
| 147 | ++ name: "go:build and limit", |
| 148 | ++ expr: "//go:build " + strings.Repeat("a && ", maxSize+2), |
| 149 | ++ }, |
| 150 | ++ { |
| 151 | ++ name: "go:build and depth limit", |
| 152 | ++ expr: "//go:build " + strings.Repeat("(a &&", maxSize+2), |
| 153 | ++ }, |
| 154 | ++ { |
| 155 | ++ name: "go:build or depth limit", |
| 156 | ++ expr: "//go:build " + strings.Repeat("(a ||", maxSize+2), |
| 157 | ++ }, |
| 158 | ++ } { |
| 159 | ++ t.Run(tc.name, func(t *testing.T) { |
| 160 | ++ _, err := Parse(tc.expr) |
| 161 | ++ if err == nil { |
| 162 | ++ t.Error("expression did not trigger limit") |
| 163 | ++ } else if syntaxErr, ok := err.(*SyntaxError); !ok || syntaxErr.Err != "build expression too large" { |
| 164 | ++ if !ok { |
| 165 | ++ t.Errorf("unexpected error: %v", err) |
| 166 | ++ } else { |
| 167 | ++ t.Errorf("unexpected syntax error: %s", syntaxErr.Err) |
| 168 | ++ } |
| 169 | ++ } |
| 170 | ++ }) |
| 171 | ++ } |
| 172 | ++} |
| 173 | ++ |
| 174 | ++func TestPlusSizeLimits(t *testing.T) { |
| 175 | ++ maxOldSize := 100 |
| 176 | ++ for _, tc := range []struct { |
| 177 | ++ name string |
| 178 | ++ expr string |
| 179 | ++ }{ |
| 180 | ++ { |
| 181 | ++ name: "+build or limit", |
| 182 | ++ expr: "// +build " + strings.Repeat("a ", maxOldSize+2), |
| 183 | ++ }, |
| 184 | ++ { |
| 185 | ++ name: "+build and limit", |
| 186 | ++ expr: "// +build " + strings.Repeat("a,", maxOldSize+2), |
| 187 | ++ }, |
| 188 | ++ } { |
| 189 | ++ t.Run(tc.name, func(t *testing.T) { |
| 190 | ++ _, err := Parse(tc.expr) |
| 191 | ++ if err == nil { |
| 192 | ++ t.Error("expression did not trigger limit") |
| 193 | ++ } else if err != errComplex { |
| 194 | ++ t.Errorf("unexpected error: got %q, want %q", err, errComplex) |
| 195 | ++ } |
| 196 | ++ }) |
| 197 | ++ } |
| 198 | ++} |
0 commit comments