Skip to content

Commit

Permalink
planner/core,session: fix privilege check for update (pingcap#8376)
Browse files Browse the repository at this point in the history
In the statement: "update t1,t2 set t1.id = t2.id"
TiDB should check update privilege for t1 and select privilege for t2,
Fix a bug that it checks update privilege for both t1 and t2
  • Loading branch information
tiancaiamao authored and yu34po committed Nov 22, 2018
1 parent 43d4cd8 commit e924347
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
6 changes: 5 additions & 1 deletion planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2096,7 +2096,7 @@ func (b *PlanBuilder) buildUpdate(update *ast.UpdateStmt) (Plan, error) {
if dbName == "" {
dbName = b.ctx.GetSessionVars().CurrentDB
}
b.visitInfo = appendVisitInfo(b.visitInfo, mysql.UpdatePriv, dbName, t.Name.L, "")
b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, dbName, t.Name.L, "")
}

if sel.Where != nil {
Expand Down Expand Up @@ -2208,6 +2208,10 @@ func (b *PlanBuilder) buildUpdateLists(tableList []*ast.TableName, list []*ast.A
p = np
newList = append(newList, &expression.Assignment{Col: col, Expr: newExpr})
}
for _, assign := range newList {
col := assign.Col
b.visitInfo = appendVisitInfo(b.visitInfo, mysql.UpdatePriv, col.DBName.L, col.TblName.L, "")
}
return newList, p, nil
}

Expand Down
28 changes: 28 additions & 0 deletions session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package session_test

import (
"fmt"
"strings"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -2226,3 +2227,30 @@ func (s *testSessionSuite) TestSetGroupConcatMaxLen(c *C) {
_, err = tk.Exec("set @@group_concat_max_len='hello'")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue, Commentf("err %v", err))
}

func (s *testSessionSuite) TestUpdatePrivilege(c *C) {
tk := testkit.NewTestKitWithInit(c, s.store)
tk.MustExec("drop table if exists t1, t2;")
tk.MustExec("create table t1 (id int);")
tk.MustExec("create table t2 (id int);")
tk.MustExec("insert into t1 values (1);")
tk.MustExec("insert into t2 values (2);")
tk.MustExec("create user xxx;")
tk.MustExec("grant all on test.t1 to xxx;")
tk.MustExec("grant select on test.t2 to xxx;")
tk.MustExec("flush privileges;")

tk1 := testkit.NewTestKitWithInit(c, s.store)
c.Assert(tk1.Se.Auth(&auth.UserIdentity{Username: "xxx", Hostname: "localhost"},
[]byte(""),
[]byte("")), IsTrue)

_, err := tk1.Exec("update t2 set id = 666 where id = 1;")
c.Assert(err, NotNil)
c.Assert(strings.Contains(err.Error(), "privilege check fail"), IsTrue)

// Cover a bug that t1 and t2 both require update privilege.
// In fact, the privlege check for t1 should be update, and for t2 should be select.
_, err = tk1.Exec("update t1,t2 set t1.id = t2.id;")
c.Assert(err, IsNil)
}

0 comments on commit e924347

Please sign in to comment.