Skip to content

[fix](nereids) reject correlated subqueries in CREATE ROW POLICY USING clause - #67540

Open
sinhaparth5 wants to merge 1 commit into
apache:masterfrom
sinhaparth5:fix/row-policy-correlated-subquery
Open

[fix](nereids) reject correlated subqueries in CREATE ROW POLICY USING clause#67540
sinhaparth5 wants to merge 1 commit into
apache:masterfrom
sinhaparth5:fix/row-policy-correlated-subquery

Conversation

@sinhaparth5

Copy link
Copy Markdown

What problem does this PR solve?

Issue Number: close#62729

Problem Summary:

This is a security issue. CREATE ROW POLICY with a correlated EXISTS subquery in the USING clause returns success but the policy is never stored or enforced, so an admin can believe a row-level filter is active when every row is actually visible.

Exists (and the other SubqueryExpr subclasses, InSubquery and ScalarSubquery) implement LeafExpression, so their inner subquery plan is not a child in the expression tree. CreatePolicyCommand.validate() checks that every column referenced in the USING clause exists on the target table by walking the predicate with wherePredicate.foreach(), but foreach() never descends into a LeafExpression's subquery plan, so anything inside an EXISTS is invisible to it. The predicate gets stored as written. Later, when the policy is injected into a query plan, the subquery's reference back to the outer table can't be resolved there (the outer table is out of scope by that point), and the policy silently becomes a no-op instead of raising an error.

Fix: when validate()'s walk reaches a SubqueryExpr, manually walk into its query plan (via Plan.getExpressions() and Plan.foreach(), since foreach() won't do this on its own) looking for an UnboundSlot whose column exists on the table the policy is being created on. If one turns up, reject the CREATE ROW POLICY statement immediately instead of storing a policy that will never fire.

This is Option A from the issue (reject correlated subqueries at DDL time), chosen over Option B (make correlation actually resolve end to end) as the safer, lower-risk fix. It's name-based rather than a real bind, so it can also reject a subquery whose own local table happens to share a column name with the outer table even when that reference would resolve locally (a self-join, for example). That's an intentional trade-off: a false rejection here is safe, a silently dropped security policy is not.

Also made CreatePolicyCommand.validate() public (it was private) so the added test can call it directly. DropRowPolicyCommand.validate() is already public, so this just matches its sibling.

Release note

Reject CREATE ROW POLICY statements whose USING clause contains a subquery correlated to the outer table, instead of silently accepting the statement and never enforcing the policy.

Check List (For Author)

  • Test
    • Unit Test

Added CreatePolicyCommandTest.java with three cases, run via mvn -pl fe-common,fe-core -am test -Dtest=CreatePolicyCommandTest: a plain predicate (allowed), an uncorrelated EXISTS (allowed), and a correlated EXISTS referencing the outer table (rejected). All 3 pass.

  • Behavior changed:

    • Yes. CREATE ROW POLICY now rejects a USING clause whose subquery is correlated to the outer table (previously accepted silently, with the policy never enforced).
  • Does this need documentation?

    • No.

…G clause
CREATE ROW POLICY with a correlated EXISTS subquery in the USING
clause silently succeeded while storing nothing, so an admin could
believe a row-level filter was active when it wasn't enforced at
all.
Exists (and the other SubqueryExpr subclasses, InSubquery and
ScalarSubquery) implement LeafExpression, so their inner subquery
plan isn't a child in the expression tree. CreatePolicyCommand
.validate() checks that every column referenced in the USING clause
exists on the target table by walking the predicate with
wherePredicate.foreach(), but foreach() never descends into a
LeafExpression's subquery plan, so everything inside an EXISTS was
invisible to it. The predicate got stored as written. Later, when
the policy is injected into a query plan, the subquery's reference
back to the outer table can't be resolved there (the outer table is
out of scope by that point), and the policy quietly turns into a
no-op instead of raising an error.
Fix: when validate()'s walk reaches a SubqueryExpr, manually walk
into its query plan (via Plan.getExpressions() and Plan.foreach(),
since foreach() won't do this on its own) looking for an UnboundSlot
whose column exists on the table the policy is being created on. If
one turns up, reject the CREATE ROW POLICY right away instead of
storing a policy that will never fire.
This is Option A from the issue: reject correlated subqueries at DDL
time, rather than Option B (make correlation actually resolve end to
end). It's name-based, not a real bind, so it can also reject a
subquery whose own local table happens to share a column name with
the outer table even when that reference would resolve locally (a
self-join, say). That's an accepted trade-off: a false rejection
here is safe, a silently dropped security policy is not.
Added CreatePolicyCommandTest.java: a plain predicate (allowed), an
uncorrelated EXISTS (allowed), and a correlated EXISTS referencing
the outer table (rejected).
Also made CreatePolicyCommand.validate() public (it was private) so
the test can call it directly. DropRowPolicyCommand.validate() is
already public, so this just matches its sibling.
Fixesapache#62729
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] CREATE ROW POLICY with correlated EXISTS subquery silently succeeds but policy is never enforced

2 participants

@sinhaparth5@hello-stephen