sinhaparth5 opened a new pull request, #67540:
URL: https://github.com/apache/doris/pull/67540
### 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
- [x] 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:
- [x] 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
