|
| 1 | +From 7884928e4539de0a800414e2c68a2912a386344d Mon Sep 17 00:00:00 2001 |
| 2 | +From: akhila-guruju <v-guakhila@microsoft.com> |
| 3 | +Date: Tue, 10 Jun 2025 07:21:09 +0000 |
| 4 | +Subject: [PATCH] Address CVE-2025-46569 |
| 5 | + |
| 6 | +Upstream Patch Reference: https://github.com/open-policy-agent/opa/commit/ad2063247a14711882f18c387a511fc8094aa79c |
| 7 | + |
| 8 | +--- |
| 9 | + server/server.go | 92 +++++++++++++++---- |
| 10 | + server/server_test.go | 150 ++++++++++++++++++++++++++++++- |
| 11 | + test/e2e/metrics/metrics_test.go | 1 - |
| 12 | + 3 files changed, 226 insertions(+), 17 deletions(-) |
| 13 | + |
| 14 | +diff --git a/server/server.go b/server/server.go |
| 15 | +index 64eeaa6..e5ab876 100644 |
| 16 | +--- a/server/server.go |
| 17 | ++++ b/server/server.go |
| 18 | +@@ -1135,19 +1135,23 @@ func (s *Server) v0QueryPath(w http.ResponseWriter, r *http.Request, urlPath str |
| 19 | + } |
| 20 | + |
| 21 | + if len(rs) == 0 { |
| 22 | +- ref := stringPathToDataRef(urlPath) |
| 23 | ++ ref, err := stringPathToDataRef(urlPath) |
| 24 | ++ if err != nil { |
| 25 | ++ writer.Error(w, http.StatusBadRequest, types.NewErrorV1(types.CodeInvalidParameter, "invalid path: %v", err)) |
| 26 | ++ return |
| 27 | ++ } |
| 28 | + |
| 29 | + var messageType = types.MsgMissingError |
| 30 | + if len(s.getCompiler().GetRulesForVirtualDocument(ref)) > 0 { |
| 31 | + messageType = types.MsgFoundUndefinedError |
| 32 | + } |
| 33 | +- err := types.NewErrorV1(types.CodeUndefinedDocument, fmt.Sprintf("%v: %v", messageType, ref)) |
| 34 | +- if err := logger.Log(ctx, txn, urlPath, "", goInput, input, nil, ndbCache, err, m); err != nil { |
| 35 | ++ errV1 := types.NewErrorV1(types.CodeUndefinedDocument, "%v: %v", messageType, ref) |
| 36 | ++ if err := logger.Log(ctx, txn, urlPath, "", goInput, input, nil, ndbCache, errV1, m); err != nil { |
| 37 | + writer.ErrorAuto(w, err) |
| 38 | + return |
| 39 | + } |
| 40 | + |
| 41 | +- writer.Error(w, http.StatusNotFound, err) |
| 42 | ++ writer.Error(w, http.StatusNotFound, errV1) |
| 43 | + return |
| 44 | + } |
| 45 | + err = logger.Log(ctx, txn, urlPath, "", goInput, input, &rs[0].Expressions[0].Value, ndbCache, nil, m) |
| 46 | +@@ -1306,10 +1310,15 @@ func (s *Server) unversionedGetHealthWithPolicy(w http.ResponseWriter, r *http.R |
| 47 | + vars := mux.Vars(r) |
| 48 | + urlPath := vars["path"] |
| 49 | + healthDataPath := fmt.Sprintf("/system/health/%s", urlPath) |
| 50 | +- healthDataPath = stringPathToDataRef(healthDataPath).String() |
| 51 | ++ |
| 52 | ++ healthDataPathQuery, err := stringPathToQuery(healthDataPath) |
| 53 | ++ if err != nil { |
| 54 | ++ writer.Error(w, http.StatusBadRequest, types.NewErrorV1(types.CodeInvalidParameter, "invalid path: %v", err)) |
| 55 | ++ return |
| 56 | ++ } |
| 57 | + |
| 58 | + rego := rego.New( |
| 59 | +- rego.Query(healthDataPath), |
| 60 | ++ rego.ParsedQuery(healthDataPathQuery), |
| 61 | + rego.Compiler(s.getCompiler()), |
| 62 | + rego.Store(s.store), |
| 63 | + rego.Input(input), |
| 64 | +@@ -1324,7 +1333,7 @@ func (s *Server) unversionedGetHealthWithPolicy(w http.ResponseWriter, r *http.R |
| 65 | + } |
| 66 | + |
| 67 | + if len(rs) == 0 { |
| 68 | +- writeHealthResponse(w, fmt.Errorf("health check (%v) was undefined", healthDataPath)) |
| 69 | ++ writeHealthResponse(w, fmt.Errorf("health check (%v) was undefined", healthDataPathQuery)) |
| 70 | + return |
| 71 | + } |
| 72 | + |
| 73 | +@@ -1334,7 +1343,7 @@ func (s *Server) unversionedGetHealthWithPolicy(w http.ResponseWriter, r *http.R |
| 74 | + return |
| 75 | + } |
| 76 | + |
| 77 | +- writeHealthResponse(w, fmt.Errorf("health check (%v) returned unexpected value", healthDataPath)) |
| 78 | ++ writeHealthResponse(w, fmt.Errorf("health check (%v) returned unexpected value", healthDataPathQuery)) |
| 79 | + } |
| 80 | + |
| 81 | + func writeHealthResponse(w http.ResponseWriter, err error) { |
| 82 | +@@ -2551,12 +2560,15 @@ func (s *Server) makeRego(ctx context.Context, |
| 83 | + tracer topdown.QueryTracer, |
| 84 | + opts []func(*rego.Rego), |
| 85 | + ) (*rego.Rego, error) { |
| 86 | +- queryPath := stringPathToDataRef(urlPath).String() |
| 87 | ++ query, err := stringPathToQuery(urlPath) |
| 88 | ++ if err != nil { |
| 89 | ++ return nil, types.NewErrorV1(types.CodeInvalidParameter, "invalid path: %v", err) |
| 90 | ++ } |
| 91 | + |
| 92 | + opts = append( |
| 93 | + opts, |
| 94 | + rego.Transaction(txn), |
| 95 | +- rego.Query(queryPath), |
| 96 | ++ rego.ParsedQuery(query), |
| 97 | + rego.ParsedInput(input), |
| 98 | + rego.Metrics(m), |
| 99 | + rego.QueryTracer(tracer), |
| 100 | +@@ -2571,6 +2583,43 @@ func (s *Server) makeRego(ctx context.Context, |
| 101 | + return rego.New(opts...), nil |
| 102 | + } |
| 103 | + |
| 104 | ++func stringPathToQuery(urlPath string) (ast.Body, error) { |
| 105 | ++ ref, err := stringPathToDataRef(urlPath) |
| 106 | ++ if err != nil { |
| 107 | ++ return nil, err |
| 108 | ++ } |
| 109 | ++ |
| 110 | ++ return parseRefQuery(ref.String()) |
| 111 | ++} |
| 112 | ++ |
| 113 | ++// parseRefQuery parses a string into a query ast.Body. |
| 114 | ++// The resulting query must be comprised of a single ref, or an error will be returned. |
| 115 | ++func parseRefQuery(str string) (ast.Body, error) { |
| 116 | ++ query, err := ast.ParseBody(str) |
| 117 | ++ if err != nil { |
| 118 | ++ return nil, errors.New("failed to parse query") |
| 119 | ++ } |
| 120 | ++ |
| 121 | ++ // assert the query is exactly one statement |
| 122 | ++ if l := len(query); l == 0 { |
| 123 | ++ return nil, errors.New("no ref") |
| 124 | ++ } else if l > 1 { |
| 125 | ++ return nil, errors.New("complex query") |
| 126 | ++ } |
| 127 | ++ |
| 128 | ++ // assert the single statement is a lone ref |
| 129 | ++ expr := query[0] |
| 130 | ++ switch t := expr.Terms.(type) { |
| 131 | ++ case *ast.Term: |
| 132 | ++ switch t.Value.(type) { |
| 133 | ++ case ast.Ref: |
| 134 | ++ return query, nil |
| 135 | ++ } |
| 136 | ++ } |
| 137 | ++ |
| 138 | ++ return nil, errors.New("complex query") |
| 139 | ++} |
| 140 | ++ |
| 141 | + func (s *Server) prepareV1PatchSlice(root string, ops []types.PatchV1) (result []patchImpl, err error) { |
| 142 | + |
| 143 | + root = "/" + strings.Trim(root, "/") |
| 144 | +@@ -2678,23 +2727,36 @@ func (s *Server) updateNDCache(enabled bool) { |
| 145 | + s.ndbCacheEnabled = enabled |
| 146 | + } |
| 147 | + |
| 148 | +-func stringPathToDataRef(s string) (r ast.Ref) { |
| 149 | ++func stringPathToDataRef(s string) (ast.Ref, error) { |
| 150 | + result := ast.Ref{ast.DefaultRootDocument} |
| 151 | +- return append(result, stringPathToRef(s)...) |
| 152 | ++ r, err := stringPathToRef(s) |
| 153 | ++ if err != nil { |
| 154 | ++ return nil, err |
| 155 | ++ } |
| 156 | ++ return append(result, r...), nil |
| 157 | + } |
| 158 | + |
| 159 | +-func stringPathToRef(s string) (r ast.Ref) { |
| 160 | ++func stringPathToRef(s string) (ast.Ref, error) { |
| 161 | ++ r := ast.Ref{} |
| 162 | ++ |
| 163 | + if len(s) == 0 { |
| 164 | +- return r |
| 165 | ++ return r, nil |
| 166 | + } |
| 167 | ++ |
| 168 | + p := strings.Split(s, "/") |
| 169 | + for _, x := range p { |
| 170 | + if x == "" { |
| 171 | + continue |
| 172 | + } |
| 173 | ++ |
| 174 | + if y, err := url.PathUnescape(x); err == nil { |
| 175 | + x = y |
| 176 | + } |
| 177 | ++ |
| 178 | ++ if strings.Contains(x, "\"") { |
| 179 | ++ return nil, fmt.Errorf("invalid ref term '%s'", x) |
| 180 | ++ } |
| 181 | ++ |
| 182 | + i, err := strconv.Atoi(x) |
| 183 | + if err != nil { |
| 184 | + r = append(r, ast.StringTerm(x)) |
| 185 | +@@ -2702,7 +2764,7 @@ func stringPathToRef(s string) (r ast.Ref) { |
| 186 | + r = append(r, ast.IntNumberTerm(i)) |
| 187 | + } |
| 188 | + } |
| 189 | +- return r |
| 190 | ++ return r, nil |
| 191 | + } |
| 192 | + |
| 193 | + func validateQuery(query string) (ast.Body, error) { |
| 194 | +diff --git a/server/server_test.go b/server/server_test.go |
| 195 | +index 9a827af..8de136e 100644 |
| 196 | +--- a/server/server_test.go |
| 197 | ++++ b/server/server_test.go |
| 198 | +@@ -2736,7 +2736,6 @@ func TestDataMetricsEval(t *testing.T) { |
| 199 | + "counter_disk_read_keys", |
| 200 | + "counter_disk_read_bytes", |
| 201 | + "timer_rego_input_parse_ns", |
| 202 | +- "timer_rego_query_parse_ns", |
| 203 | + "timer_rego_query_compile_ns", |
| 204 | + "timer_rego_query_eval_ns", |
| 205 | + "timer_server_handler_ns", |
| 206 | +@@ -5739,3 +5738,152 @@ func zipString(input string) []byte { |
| 207 | + } |
| 208 | + return b.Bytes() |
| 209 | + } |
| 210 | ++ |
| 211 | ++func TestStringPathToDataRef(t *testing.T) { |
| 212 | ++ t.Parallel() |
| 213 | ++ |
| 214 | ++ cases := []struct { |
| 215 | ++ note string |
| 216 | ++ path string |
| 217 | ++ expRef string |
| 218 | ++ expErr string |
| 219 | ++ }{ |
| 220 | ++ {path: "foo", expRef: `data.foo`}, |
| 221 | ++ {path: "foo/", expRef: `data.foo`}, |
| 222 | ++ {path: "foo/bar", expRef: `data.foo.bar`}, |
| 223 | ++ {path: "foo/bar/", expRef: `data.foo.bar`}, |
| 224 | ++ {path: "foo/../bar", expRef: `data.foo[".."].bar`}, |
| 225 | ++ |
| 226 | ++ // Path injection attack |
| 227 | ++ // url path: `foo%22%5D%3Bmalicious_call%28%29%3Bx%3D%5B%22` |
| 228 | ++ // url decoded: `foo"];malicious_call();x=["` |
| 229 | ++ // data ref .String(): `data.foo["\"];malicious_call();x=[\""]` |
| 230 | ++ // Above attack is mitigated by rejecting any ref component containing string terminators (`"`). |
| 231 | ++ { |
| 232 | ++ note: "string terminals inside ref term", |
| 233 | ++ path: "foo%22%5D%3Bmalicious_call%28%29%3Bx%3D%5B%22", // foo"];malicious_call();x=[" |
| 234 | ++ expErr: `invalid ref term 'foo"];malicious_call();x=["'`, |
| 235 | ++ }, |
| 236 | ++ } |
| 237 | ++ |
| 238 | ++ for _, tc := range cases { |
| 239 | ++ note := tc.note |
| 240 | ++ if note == "" { |
| 241 | ++ note = strings.ReplaceAll(tc.path, "/", "_") |
| 242 | ++ } |
| 243 | ++ |
| 244 | ++ t.Run(note, func(t *testing.T) { |
| 245 | ++ ref, err := stringPathToDataRef(tc.path) |
| 246 | ++ |
| 247 | ++ if tc.expRef != "" { |
| 248 | ++ if err != nil { |
| 249 | ++ t.Fatalf("Expected ref:\n\n%s\n\nbut got error:\n\n%s", tc.expRef, err) |
| 250 | ++ } |
| 251 | ++ if refStr := ref.String(); refStr != tc.expRef { |
| 252 | ++ t.Fatalf("Expected ref:\n\n%s\n\nbut got:\n\n%s", tc.expRef, refStr) |
| 253 | ++ } |
| 254 | ++ } |
| 255 | ++ |
| 256 | ++ if tc.expErr != "" { |
| 257 | ++ if ref != nil { |
| 258 | ++ t.Fatalf("Expected error:\n\n%s\n\nbut got ref:\n\n%s", tc.expErr, ref.String()) |
| 259 | ++ } |
| 260 | ++ if errStr := err.Error(); errStr != tc.expErr { |
| 261 | ++ t.Fatalf("Expected error:\n\n%s\n\nbut got ref:\n\n%s", tc.expErr, errStr) |
| 262 | ++ } |
| 263 | ++ } |
| 264 | ++ }) |
| 265 | ++ } |
| 266 | ++} |
| 267 | ++ |
| 268 | ++func TestParseRefQuery(t *testing.T) { |
| 269 | ++ t.Parallel() |
| 270 | ++ |
| 271 | ++ cases := []struct { |
| 272 | ++ note string |
| 273 | ++ raw string |
| 274 | ++ expBody ast.Body |
| 275 | ++ expErr string |
| 276 | ++ }{ |
| 277 | ++ { |
| 278 | ++ note: "unparseable", |
| 279 | ++ raw: `}abc{`, |
| 280 | ++ expErr: "failed to parse query", |
| 281 | ++ }, |
| 282 | ++ { |
| 283 | ++ note: "empty", |
| 284 | ++ raw: ``, |
| 285 | ++ expErr: "no ref", |
| 286 | ++ }, |
| 287 | ++ { |
| 288 | ++ note: "single ref", |
| 289 | ++ raw: `data.foo.bar`, |
| 290 | ++ expBody: ast.MustParseBody(`data.foo.bar`), |
| 291 | ++ }, |
| 292 | ++ { |
| 293 | ++ note: "multiple refs,';' separated", |
| 294 | ++ raw: `data.foo.bar;data.baz.qux`, |
| 295 | ++ expErr: "complex query", |
| 296 | ++ }, |
| 297 | ++ { |
| 298 | ++ note: "multiple refs,newline separated", |
| 299 | ++ raw: `data.foo.bar |
| 300 | ++data.baz.qux`, |
| 301 | ++ expErr: "complex query", |
| 302 | ++ }, |
| 303 | ++ { |
| 304 | ++ note: "single ref + call", |
| 305 | ++ raw: `data.foo.bar;data.baz.qux()`, |
| 306 | ++ expErr: "complex query", |
| 307 | ++ }, |
| 308 | ++ { |
| 309 | ++ note: "single ref + assignment", |
| 310 | ++ raw: `data.foo.bar;x := 42`, |
| 311 | ++ expErr: "complex query", |
| 312 | ++ }, |
| 313 | ++ { |
| 314 | ++ note: "single call", |
| 315 | ++ raw: `data.foo.bar()`, |
| 316 | ++ expErr: "complex query", |
| 317 | ++ }, |
| 318 | ++ { |
| 319 | ++ note: "single assignment", |
| 320 | ++ raw: `x := 42`, |
| 321 | ++ expErr: "complex query", |
| 322 | ++ }, |
| 323 | ++ { |
| 324 | ++ note: "single unification", |
| 325 | ++ raw: `x = 42`, |
| 326 | ++ expErr: "complex query", |
| 327 | ++ }, |
| 328 | ++ { |
| 329 | ++ note: "single equality", |
| 330 | ++ raw: `x == 42`, |
| 331 | ++ expErr: "complex query", |
| 332 | ++ }, |
| 333 | ++ } |
| 334 | ++ |
| 335 | ++ for _, tc := range cases { |
| 336 | ++ t.Run(tc.note, func(t *testing.T) { |
| 337 | ++ body, err := parseRefQuery(tc.raw) |
| 338 | ++ |
| 339 | ++ if tc.expBody != nil { |
| 340 | ++ if err != nil { |
| 341 | ++ t.Fatalf("Expected body:\n\n%s\n\nbut got error:\n\n%s", tc.expBody, err) |
| 342 | ++ } |
| 343 | ++ if body.String() != tc.expBody.String() { |
| 344 | ++ t.Fatalf("Expected body:\n\n%s\n\nbut got:\n\n%s", tc.expBody, body.String()) |
| 345 | ++ } |
| 346 | ++ } |
| 347 | ++ |
| 348 | ++ if tc.expErr != "" { |
| 349 | ++ if body != nil { |
| 350 | ++ t.Fatalf("Expected error:\n\n%s\n\nbut got body:\n\n%s", tc.expErr, body.String()) |
| 351 | ++ } |
| 352 | ++ if errStr := err.Error(); errStr != tc.expErr { |
| 353 | ++ t.Fatalf("Expected error:\n\n%s\n\nbut got body:\n\n%s", tc.expErr, errStr) |
| 354 | ++ } |
| 355 | ++ } |
| 356 | ++ }) |
| 357 | ++ } |
| 358 | ++} |
| 359 | +diff --git a/test/e2e/metrics/metrics_test.go b/test/e2e/metrics/metrics_test.go |
| 360 | +index e067909..e90d8fb 100644 |
| 361 | +--- a/test/e2e/metrics/metrics_test.go |
| 362 | ++++ b/test/e2e/metrics/metrics_test.go |
| 363 | +@@ -211,7 +211,6 @@ func assertDataInstrumentationMetricsInMap(t *testing.T, includeCompile bool, me |
| 364 | + "timer_server_handler_ns", |
| 365 | + } |
| 366 | + compileStageKeys := []string{ |
| 367 | +- "timer_rego_query_parse_ns", |
| 368 | + "timer_rego_query_compile_ns", |
| 369 | + "timer_query_compile_stage_build_comprehension_index_ns", |
| 370 | + "timer_query_compile_stage_check_safety_ns", |
| 371 | +-- |
| 372 | +2.45.2 |
| 373 | + |
0 commit comments