From 14ff938d8c6497ab0be07d43dff277a01481758c Mon Sep 17 00:00:00 2001
From: bb7133 <bb7133@gmail.com>
Date: Thu, 21 Nov 2024 21:40:44 -0800
Subject: [PATCH] planner/core: fix a wrong privilege check for CTE & UPDATE
 statement (#57430)

close pingcap/tidb#53490
---
 pkg/planner/core/logical_plan_builder.go      | 11 +++++---
 pkg/planner/core/planbuilder.go               |  4 ++-
 .../r/privilege/privileges.result             | 16 +++++++++++
 .../t/privilege/privileges.test               | 27 +++++++++++++++++++
 4 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/pkg/planner/core/logical_plan_builder.go b/pkg/planner/core/logical_plan_builder.go
index 6e760c868e0ff..2b24d1b5bfb8c 100644
--- a/pkg/planner/core/logical_plan_builder.go
+++ b/pkg/planner/core/logical_plan_builder.go
@@ -5502,7 +5502,10 @@ func (b *PlanBuilder) buildUpdate(ctx context.Context, update *ast.UpdateStmt) (
 		if dbName == "" {
 			dbName = b.ctx.GetSessionVars().CurrentDB
 		}
-		b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, dbName, t.Name.L, "", nil)
+		// Avoid adding CTE table to the SELECT privilege list, maybe we have better way to do this?
+		if _, ok := b.nameMapCTE[t.Name.L]; !ok {
+			b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, dbName, t.Name.L, "", nil)
+		}
 	}
 
 	oldSchemaLen := p.Schema().Len()
@@ -7406,12 +7409,12 @@ func (b *PlanBuilder) genCTETableNameForError() string {
 
 func (b *PlanBuilder) buildWith(ctx context.Context, w *ast.WithClause) ([]*cteInfo, error) {
 	// Check CTE name must be unique.
-	nameMap := make(map[string]struct{})
+	b.nameMapCTE = make(map[string]struct{})
 	for _, cte := range w.CTEs {
-		if _, ok := nameMap[cte.Name.L]; ok {
+		if _, ok := b.nameMapCTE[cte.Name.L]; ok {
 			return nil, plannererrors.ErrNonUniqTable
 		}
-		nameMap[cte.Name.L] = struct{}{}
+		b.nameMapCTE[cte.Name.L] = struct{}{}
 	}
 	ctes := make([]*cteInfo, 0, len(w.CTEs))
 	for _, cte := range w.CTEs {
diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go
index 24b2604453542..100d70e810789 100644
--- a/pkg/planner/core/planbuilder.go
+++ b/pkg/planner/core/planbuilder.go
@@ -269,8 +269,10 @@ type PlanBuilder struct {
 	allocIDForCTEStorage        int
 	buildingRecursivePartForCTE bool
 	buildingCTE                 bool
-	//Check whether the current building query is a CTE
+	// Check whether the current building query is a CTE
 	isCTE bool
+	// CTE table name in lower case, it can be nil
+	nameMapCTE map[string]struct{}
 
 	// subQueryCtx and subQueryHintFlags are for handling subquery related hints.
 	// Note: "subquery" here only contains subqueries that are handled by the expression rewriter, i.e., [NOT] IN,
diff --git a/tests/integrationtest/r/privilege/privileges.result b/tests/integrationtest/r/privilege/privileges.result
index d42b1ca3d44eb..090176fbb85a4 100644
--- a/tests/integrationtest/r/privilege/privileges.result
+++ b/tests/integrationtest/r/privilege/privileges.result
@@ -647,3 +647,19 @@ ADMIN SHOW SLOW TOP ALL 3;
 Error 8121 (HY000): privilege check for 'Super' fail
 ADMIN ALTER DDL JOBS 10 THREAD = 3, BATCH_SIZE = 100, MAX_WRITE_SPEED = '10MiB';
 Error 8121 (HY000): privilege check for 'Super' fail
+create table privilege__privileges.tt1 (id bigint,pid bigint,name varchar(20),fullname varchar(20));
+insert into privilege__privileges.tt1 values (1,null,'a',''),(2,1,'b',''),(3,2,'c','');
+CREATE USER u53490;
+GRANT USAGE ON *.* TO 'u53490';
+GRANT SELECT,INSERT,UPDATE,DELETE,CREATE,DROP,CREATE ROUTINE,ALTER ROUTINE,ALTER,EXECUTE,INDEX,CREATE VIEW,SHOW VIEW ON privilege__privileges.* TO 'u53490';
+with t_f as (
+select id,pid,name,'AAA' fullname from privilege__privileges.tt1 )
+update privilege__privileges.tt1 inner join t_f
+set tt1.fullname=t_f.fullname
+where tt1.id=t_f.id;
+with t_f as (
+select id,pid,name,'AAA' fullname from privilege__privileges.tt1 )
+update privilege__privileges.tt1 inner join t_f
+set t_f.fullname=t_f.fullname
+where tt1.id=t_f.id;
+Error 1288 (HY000): The target table t_f of the UPDATE is not updatable
diff --git a/tests/integrationtest/t/privilege/privileges.test b/tests/integrationtest/t/privilege/privileges.test
index ad3b1a7bbb0e1..a99dffc261953 100644
--- a/tests/integrationtest/t/privilege/privileges.test
+++ b/tests/integrationtest/t/privilege/privileges.test
@@ -871,3 +871,30 @@ ADMIN ALTER DDL JOBS 10 THREAD = 3, BATCH_SIZE = 100, MAX_WRITE_SPEED = '10MiB';
 
 disconnect without_super;
 connection default;
+
+# TestIssue53490
+create table privilege__privileges.tt1 (id bigint,pid bigint,name varchar(20),fullname varchar(20));
+insert into privilege__privileges.tt1 values (1,null,'a',''),(2,1,'b',''),(3,2,'c','');
+
+CREATE USER u53490;
+GRANT USAGE ON *.* TO 'u53490';
+GRANT SELECT,INSERT,UPDATE,DELETE,CREATE,DROP,CREATE ROUTINE,ALTER ROUTINE,ALTER,EXECUTE,INDEX,CREATE VIEW,SHOW VIEW ON privilege__privileges.* TO 'u53490';
+
+connect (u53490,localhost,u53490,,);
+connection u53490;
+
+with t_f as (
+         select id,pid,name,'AAA' fullname from privilege__privileges.tt1 )
+         update privilege__privileges.tt1 inner join t_f
+         set tt1.fullname=t_f.fullname
+         where tt1.id=t_f.id;
+
+-- error 1288
+with t_f as (
+         select id,pid,name,'AAA' fullname from privilege__privileges.tt1 )
+         update privilege__privileges.tt1 inner join t_f
+         set t_f.fullname=t_f.fullname
+         where tt1.id=t_f.id;
+
+disconnect u53490;
+connection default;