Skip to content

Commit 98dec70

Browse files
smith1000geshengli
andauthored
[fix](fe) Fix Ranger column-level privilege bypass when CTE combined (apache#61741)
### What problem does this PR solve? Issue Number: close apache#61631 Problem Summary: When a CTE (WITH ... AS) is referenced multiple times in a JOIN query and is not inlined (due to inlineCTEReferencedThreshold), the CheckPrivileges rule does not traverse the CTE producer subtree because LogicalCTEConsumer is a leaf node in the plan tree. This means column-level privileges on the CTE's underlying tables are never checked, allowing users without proper column access to bypass Ranger authorization. The fix adds a `visitLogicalCTEConsumer` override in `CheckPrivileges` that explicitly retrieves the CTE producer plan (stored by `RewriteCteChildren`) and traverses it for privilege checking. The `privChecked` flag remains on `StatementContext` to preserve the view permission passthrough mechanism. ### Release note Fixed a security issue where Ranger column-level privileges could be bypassed when using CTE (WITH ... AS) combined with JOIN queries. Users without proper column access permissions could read restricted columns through CTE+JOIN patterns. ### Check List (For Author) - Test: Unit Test / Manual test (verified with Ranger 2.7.0 + Doris 4.0.2 environment) - Behavior changed: No - Does this need documentation: No --------- Co-authored-by: geshengli <geshengli@wps.cn>
1 parent 668805a commit 98dec70

2 files changed

Lines changed: 56 additions & 4 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,23 @@
2525
import org.apache.doris.nereids.exceptions.AnalysisException;
2626
import org.apache.doris.nereids.jobs.JobContext;
2727
import org.apache.doris.nereids.rules.analysis.UserAuthentication;
28+
import org.apache.doris.nereids.trees.expressions.CTEId;
2829
import org.apache.doris.nereids.trees.expressions.Slot;
2930
import org.apache.doris.nereids.trees.expressions.SlotReference;
3031
import org.apache.doris.nereids.trees.expressions.functions.table.TableValuedFunction;
3132
import org.apache.doris.nereids.trees.plans.Plan;
33+
import org.apache.doris.nereids.trees.plans.logical.LogicalCTEConsumer;
3234
import org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation;
3335
import org.apache.doris.nereids.trees.plans.logical.LogicalRelation;
3436
import org.apache.doris.nereids.trees.plans.logical.LogicalTVFRelation;
3537
import org.apache.doris.nereids.trees.plans.logical.LogicalView;
3638
import org.apache.doris.qe.ConnectContext;
3739

40+
import com.google.common.collect.ImmutableList;
3841
import com.google.common.collect.Sets;
3942
import org.roaringbitmap.RoaringBitmap;
4043

44+
import java.util.HashSet;
4145
import java.util.LinkedHashMap;
4246
import java.util.List;
4347
import java.util.Map;
@@ -46,20 +50,28 @@
4650

4751
/**
4852
* CheckPrivileges
49-
* This rule should only check once, because after check would set setPrivChecked in statementContext
53+
*
54+
* The privChecked flag is on StatementContext to support view permission passthrough:
55+
* when InlineLogicalView expands a view, the outer CheckPrivileges has already verified
56+
* the view-level permission, so the inner tables should not be re-checked.
57+
*
58+
* For CTEs, since LogicalCTEConsumer is a leaf node and the producer subtree is not
59+
* traversed by ColumnPruning, we override visitLogicalCTEConsumer to explicitly
60+
* traverse the producer plan and check privileges on its tables.
5061
*/
5162
public class CheckPrivileges extends ColumnPruning {
5263
private JobContext jobContext;
64+
private final Set<CTEId> checkedCteIds = new HashSet<>();
5365

5466
@Override
5567
public Plan rewriteRoot(Plan plan, JobContext jobContext) {
56-
// Only enter once, if repeated, the permissions of the table in the view will be checked
57-
if (jobContext.getCascadesContext().getStatementContext().isPrivChecked()) {
68+
StatementContext stmtCtx = jobContext.getCascadesContext().getStatementContext();
69+
if (stmtCtx.isPrivChecked()) {
5870
return plan;
5971
}
6072
this.jobContext = jobContext;
6173
super.rewriteRoot(plan, jobContext);
62-
jobContext.getCascadesContext().getStatementContext().setPrivChecked(true);
74+
stmtCtx.setPrivChecked(true);
6375
// don't rewrite plan, because Reorder expect no LogicalProject on LogicalJoin
6476
return plan;
6577
}
@@ -88,6 +100,19 @@ public Plan visitLogicalRelation(LogicalRelation relation, PruneContext context)
88100
return super.visitLogicalRelation(relation, context);
89101
}
90102

103+
@Override
104+
public Plan visitLogicalCTEConsumer(LogicalCTEConsumer consumer, PruneContext context) {
105+
CTEId cteId = consumer.getCteId();
106+
StatementContext stmtCtx = jobContext.getCascadesContext().getStatementContext();
107+
Plan producerPlan = stmtCtx.getCteProducerByCteId(cteId);
108+
if (producerPlan != null && checkedCteIds.add(cteId)) {
109+
PruneContext producerContext = new PruneContext(
110+
null, producerPlan.getOutputExprIdBitSet(), ImmutableList.of(), true);
111+
producerPlan.accept(this, producerContext);
112+
}
113+
return super.visitLogicalCTEConsumer(consumer, context);
114+
}
115+
91116
private Set<String> computeUsedColumns(Plan plan, RoaringBitmap requiredSlotIds) {
92117
List<Slot> outputs = plan.getOutput();
93118
Map<Integer, Slot> idToSlot = new LinkedHashMap<>(outputs.size());

fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,33 @@ public void testPrivilegesAndPolicies() throws Exception {
163163
);
164164
}
165165

166+
// test CTE with JOIN privilege checking
167+
// Verifies that column-level privileges are enforced when CTE is
168+
// referenced multiple times via JOIN (CTE won't be inlined due to
169+
// inlineCTEReferencedThreshold). CheckPrivileges.visitLogicalCTEConsumer
170+
// explicitly traverses the CTE producer plan to check privileges.
171+
{
172+
// CTE + JOIN on fully-privileged table should succeed
173+
query("WITH cte AS (SELECT id, name FROM custom_catalog.test_db.test_tbl1) "
174+
+ "SELECT a.id FROM cte a LEFT JOIN cte b ON a.id = b.id");
175+
176+
// CTE + JOIN accessing restricted column should be denied
177+
Assertions.assertThrows(AnalysisException.class, () ->
178+
query("WITH cte AS (SELECT * FROM custom_catalog.test_db.test_tbl2) "
179+
+ "SELECT a.id FROM cte a LEFT JOIN cte b ON a.id = b.id")
180+
);
181+
182+
// CTE + JOIN accessing only allowed columns should succeed
183+
query("WITH cte AS (SELECT id FROM custom_catalog.test_db.test_tbl2) "
184+
+ "SELECT a.id FROM cte a LEFT JOIN cte b ON a.id = b.id");
185+
186+
// CTE + INNER JOIN accessing restricted column should also be denied
187+
Assertions.assertThrows(AnalysisException.class, () ->
188+
query("WITH cte AS (SELECT * FROM custom_catalog.test_db.test_tbl2) "
189+
+ "SELECT a.id FROM cte a INNER JOIN cte b ON a.id = b.id")
190+
);
191+
}
192+
166193
// test row policy with data masking
167194
{
168195
Function<NamedExpression, Boolean> checkId = (NamedExpression ne) -> {

0 commit comments

Comments
 (0)