Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

infoschema: improve the security vunerability of schemata infoschema table #14003

Merged
merged 17 commits into from
Dec 13, 2019
Merged

infoschema: improve the security vunerability of schemata infoschema table #14003

merged 17 commits into from
Dec 13, 2019

Conversation

reafans
Copy link
Contributor

@reafans reafans commented Dec 10, 2019

What problem does this PR solve?

improve the security vunerability of schemata infoschema table

related to #209

What is changed and how it works?

before:new user both can get the whole data of information_schema.schemata

mysql -u root -h 127.0.0.1 -P 4000
mysql> use information_schema;
mysql> select * from schemata;
+--------------+--------------------+----------------------------+------------------------+----------+
| CATALOG_NAME | SCHEMA_NAME        | DEFAULT_CHARACTER_SET_NAME | DEFAULT_COLLATION_NAME | SQL_PATH |
+--------------+--------------------+----------------------------+------------------------+----------+
| def          | INFORMATION_SCHEMA | utf8mb4                    | utf8mb4_bin            | NULL     |
| def          | mysql              | utf8mb4                    | utf8mb4_bin            | NULL     |
| def          | PERFORMANCE_SCHEMA | utf8mb4                    | utf8mb4_bin            | NULL     |
| def          | test               | utf8mb4                    | utf8mb4_bin            | NULL     |
+--------------+--------------------+----------------------------+------------------------+----------+
4 rows in set (0.00 sec)

mysql> create user tester;
mysql -u tester -h 127.0.0.1 -P 4000
mysql> use information_schema;
mysql> select * from schemata;
+--------------+--------------------+----------------------------+------------------------+----------+
| CATALOG_NAME | SCHEMA_NAME        | DEFAULT_CHARACTER_SET_NAME | DEFAULT_COLLATION_NAME | SQL_PATH |
+--------------+--------------------+----------------------------+------------------------+----------+
| def          | INFORMATION_SCHEMA | utf8mb4                    | utf8mb4_bin            | NULL     |
| def          | mysql              | utf8mb4                    | utf8mb4_bin            | NULL     |
| def          | PERFORMANCE_SCHEMA | utf8mb4                    | utf8mb4_bin            | NULL     |
| def          | test               | utf8mb4                    | utf8mb4_bin            | NULL     |
+--------------+--------------------+----------------------------+------------------------+----------+
4 rows in set (0.00 sec)

after:the new user can only get an one row set in which SCHEMA_NAME is INFORMATION_SCHEMA.

mysql -u tester -h 127.0.0.1 -P 4000
mysql> use information_schema;
mysql> select * from schemata;
+--------------+--------------------+----------------------------+------------------------+----------+
| CATALOG_NAME | SCHEMA_NAME        | DEFAULT_CHARACTER_SET_NAME | DEFAULT_COLLATION_NAME | SQL_PATH |
+--------------+--------------------+----------------------------+------------------------+----------+
| def          | INFORMATION_SCHEMA | utf8mb4                    | utf8mb4_bin            | NULL     |
+--------------+--------------------+----------------------------+------------------------+----------+
1 row in set (0.00 sec)

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change

Side effects
None

Related changes
None

@reafans reafans added component/privilege type/enhancement The issue or PR belongs to an enhancement. labels Dec 10, 2019
@codecov
Copy link

codecov bot commented Dec 10, 2019

Codecov Report

Merging #14003 into master will decrease coverage by 0.0717%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #14003        +/-   ##
================================================
- Coverage   80.2502%   80.1784%   -0.0718%     
================================================
  Files           482        482                
  Lines        121166     120813       -353     
================================================
- Hits          97236      96866       -370     
- Misses        16205      16215        +10     
- Partials       7725       7732         +7

@reafans
Copy link
Contributor Author

reafans commented Dec 10, 2019

/run-all-tests

1 similar comment
@reafans
Copy link
Contributor Author

reafans commented Dec 10, 2019

/run-all-tests

@AilinKid
Copy link
Contributor

/run-all-tests

@reafans
Copy link
Contributor Author

reafans commented Dec 12, 2019

/run-all-tests

Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

schemataTester.MustQuery("select count(*) from information_schema.SCHEMATA;").Check(testkit.Rows("2"))
schemataTester.MustQuery("select * from information_schema.SCHEMATA;").Check(
testkit.Rows("def INFORMATION_SCHEMA utf8mb4 utf8mb4_bin <nil>", "def mysql utf8mb4 utf8mb4_bin <nil>"))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test case that querying information_schema by a user whose username is empty, which means the query is an internal SQL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the case is in 450 line of this file, a new tk and tk.MustQuery() will create a user whose username is empty.

@reafans reafans added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 13, 2019
@reafans
Copy link
Contributor Author

reafans commented Dec 13, 2019

/run-all-tests

Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve

@reafans reafans merged commit 78b8865 into pingcap:master Dec 13, 2019
@reafans reafans deleted the infoschema_schemata branch December 13, 2019 06:08
@reafans reafans removed the status/LGT1 Indicates that a PR has LGTM 1. label Dec 13, 2019
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
…table (pingcap#14003)

* add check for information_schema.schemata

* Update tables_test.go

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/privilege type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants