CVE-2022-28946: Other (NVD-CWE-Other)
value: An issue in the component ast/parser.go of Open Policy Agent v0.39.0 causes the application to incorrectly interpret every expression, causing a Denial of Service (DoS) via triggering out-of-range memory access.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 7.5
Published: 2022-05-19T19:15Z
Repository: https://github.com/open-policy-agent/opa
Language: Go
Files changed: ast/parser.go, ast/parser_test.go
Commit date: 2022-04-05 13:17:11 +0200
Commit message: ast/parser: guard against invalid domains for "some" and "every" (#4548)

These would cause a runtime exception when attempting to parse

    some internal.member_2()

This is due to a technicality in how

    some x in xs

is parsed: `x in xs` will first become `internal.member_2(x, xs)`,
and `some internal.member_2(x, xs)` is then further processed. The
assumption that there're always two (resp. three for internal.member_3)
arguments won't hold if a snippet like the one above is fed into
the parser.

Thanks to Norbert Szetei of Doyensec, @doyensec, for reporting this.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Diff: diff --git a/ast/parser.go b/ast/parser.go
index 798b46dba..1f9e54c20 100644
--- a/ast/parser.go
+++ b/ast/parser.go
@@ -895,11 +895,20 @@ func (p *Parser) parseSome() *Expr {
 	s := p.save()
 	p.scan()
 	if term := p.parseTermInfixCall(); term != nil {
 		if call, ok := term.Value.(Call); ok {
 			switch call[0].String() {
-			case Member.Name, MemberWithKey.Name: // OK
+			case Member.Name:
+				if len(call) != 3 {
+					p.illegal("illegal domain")
+					return nil
+				}
+			case MemberWithKey.Name:
+				if len(call) != 4 {
+					p.illegal("illegal domain")
+					return nil
+				}
 			default:
 				p.illegal("expected `x in xs` or `x, y in xs` expression")
 				return nil
 			}
 
@@ -970,13 +979,21 @@ func (p *Parser) parseEvery() *Expr {
 		p.illegal("expected `x[, y] in xs { ... }` expression")
 		return nil
 	}
 	switch call[0].String() {
 	case Member.Name: // x in xs
+		if len(call) != 3 {
+			p.illegal("illegal domain")
+			return nil
+		}
 		qb.Value = call[1]
 		qb.Domain = call[2]
 	case MemberWithKey.Name: // k, v in xs
+		if len(call) != 4 {
+			p.illegal("illegal domain")
+			return nil
+		}
 		qb.Key = call[1]
 		qb.Value = call[2]
 		qb.Domain = call[3]
 		if _, ok := qb.Key.Value.(Var); !ok {
 			p.illegal("expected key to be a variable")
diff --git a/ast/parser_test.go b/ast/parser_test.go
index dfe0f24a3..bba75ae77 100644
--- a/ast/parser_test.go
+++ b/ast/parser_test.go
@@ -783,10 +783,14 @@ func TestSomeDeclExpr(t *testing.T) {
 					),
 				},
 			},
 			With: []*With{{Value: ArrayTerm(), Target: NewTerm(MustParseRef("input"))}},
 		}, opts)
+
+	assertParseErrorContains(t, "invalid domain (internal.member_2)", "some internal.member_2()", "illegal domain", opts)
+	assertParseErrorContains(t, "invalid domain (internal.member_3)", "some internal.member_3()", "illegal domain", opts)
+
 }
 
 func TestEvery(t *testing.T) {
 	opts := ParserOptions{unreleasedKeywords: true, FutureKeywords: []string{"every"}}
 	assertParseOneExpr(t, "simple", "every x in xs { true }",
@@ -849,10 +853,12 @@ func TestEvery(t *testing.T) {
 		every x
 	}`,
 		"unexpected ident token: expected \\n or ; or }\n\tevery x\n", // this asserts that the tail of the error message doesn't contain a hint
 	)
 
+	assertParseErrorContains(t, "invalid domain (internal.member_2)", "every internal.member_2()", "illegal domain", opts)
+	assertParseErrorContains(t, "invalid domain (internal.member_3)", "every internal.member_3()", "illegal domain", opts)
 }
 
 func TestNestedExpressions(t *testing.T) {
 
 	n1 := IntNumberTerm(1)

Vulnerable code: 			case Member.Name, MemberWithKey.Name: // OK
Fixed code: 			case Member.Name:
				if len(call) != 3 {
					p.illegal("illegal domain")
					return nil
				}
			case MemberWithKey.Name:
				if len(call) != 4 {
					p.illegal("illegal domain")
					return nil
				}
		if len(call) != 3 {
			p.illegal("illegal domain")
			return nil
		}
		if len(call) != 4 {
			p.illegal("illegal domain")
			return nil
		}

	assertParseErrorContains(t, "invalid domain (internal.member_2)", "some internal.member_2()", "illegal domain", opts)
	assertParseErrorContains(t, "invalid domain (internal.member_3)", "some internal.member_3()", "illegal domain", opts)

	assertParseErrorContains(t, "invalid domain (internal.member_2)", "every internal.member_2()", "illegal domain", opts)
	assertParseErrorContains(t, "invalid domain (internal.member_3)", "every internal.member_3()", "illegal domain", opts)