Users with only Insert privilege can perform REPLACE on that table

[kennytm]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

As root:

create user rrrr;
create table t(a int primary key, b text);
insert into t values (1, 'aa'), (2, 'bb');
grant select, insert on t to rrrr;

As user rrrr:

replace into test.t values (1, 'oh no');

2. What did you expect to see? (Required)

User rrrr should not be able to execute the REPLACE:

ERROR 1142 (42000): DELETE command denied to user 'rrrr'@'localhost' for table 't'

3. What did you see instead (Required)

The REPLACE completed successfully:

mysql> replace into test.t values (1, 'oh no');
Query OK, 2 rows affected (0.09 sec)

The row is indeed replaced:

mysql> select * from t;
+---+-------+
| a | b     |
+---+-------+
| 1 | oh no |
| 2 | bb    |
+---+-------+

4. What is your TiDB version? (Required)

1. v4.0.12
2. a certain old master version at 8ddd41c

[lysu]

insert on duplicate also have the same problem

[kennytm]

/component privilege
/sig sql-infra

[morgo]

insert on duplicate also have the same problem

I think it is technically a different issue with insert on duplicate key update. Rationale:

• REPLACE is semantically a DELETE + INSERT.
• INSERT ON DUPLICATE is semantically an INSERT, unless there is a conflict, in which case it is converted to an UPDATE (varying permissions). See https://bugs.mysql.com/bug.php?id=30915

[kennytm]

changing these lines

tidb/planner/core/planbuilder.go

Lines 2474 to 2475 in 3d8084e

	b.visitInfo = appendVisitInfo(b.visitInfo, mysql.InsertPriv, tn.DBInfo.Name.L,
	tableInfo.Name.L, "", authErr)

to include the addition privileges when insert.IsReplace should be sufficient i guess.

not sure about OnDuplicate, sounds the check needs to be dynamic.

[tiancaiamao]

Would you like to file a PR? @kennytm

[kennytm]

@morgo

INSERT ON DUPLICATE is semantically an INSERT, unless there is a conflict, in which case it is converted to an UPDATE (varying permissions). See https://bugs.mysql.com/bug.php?id=30915

The bug report actually says

I believe that the "INSERT ... ON DUPLICATE KEY UPDATE" statement should only require INSERT privilege and in addition, UPDATE privilege for each column affected.

So even if I write INSERT INTO t VALUES (3, 'xxx') ON DUPLICATE KEY UPDATE b = 'yyy' which no conflict exists we still always get:

ERROR 1142 (42000): UPDATE command denied to user 'rrrr'@'localhost' for table 't'

that bug report is about column-level privileges (#9766) which we don't support yet. So we can just always require INSERT + UPDATE privileges in case of ON DUPLICATE KEY UPDATE.

[morgo]

that bug report is about column-level privileges (#9766) which we don't support yet. So we can just always require INSERT + UPDATE privileges in case of ON DUPLICATE KEY UPDATE.

+1. I recently fixed a bug with DELETE, where TiDB will require more privileges than MySQL. I think requiring more is fine.

[ti-srebot]

Please edit this comment or add a new comment to complete the following information

Not a bug

1. Remove the 'type/bug' label
2. Add notes to indicate why it is not a bug

Duplicate bug

1. Add the 'type/duplicate' label
2. Add the link to the original bug

Bug

Note: Make Sure that 'component', and 'severity' labels are added
Example for how to fill out the template: #20100

1. Root Cause Analysis (RCA) (optional)

2. Symptom (optional)

3. All Trigger Conditions (optional)

4. Workaround (optional)

5. Affected versions

6. Fixed versions

[kennytm]

1. Root Cause Analysis (RCA) (optional)

Privilege check is not strict enough.

2. Symptom (optional)

Users with only INSERT privilege on a table can perform REPLACE or INSERT ON DUPLICATE KEY UPDATE, allowing them to override existing data.

3. All Trigger Conditions (optional)

4. Workaround (optional)

None.

5. Affected versions

[v3.0.0:v3.0.20], [v4.0.0:v4.0.12], [v5.0.0:v5.0.0]

6. Fixed versions

v4.0.13, v5.0.1