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
Implement SHOW INDEX #49158
Implement SHOW INDEX #49158
Conversation
<< "primary_key AS expression " | ||
<< "FROM system.tables " | ||
<< "WHERE " | ||
<< "database = '" << database << "' " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SQL injection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a few injection attacks from here but either the parser rejected such queries or there was an error when the rewritten query was executed.
What worked was an attack with 1=1
, e.g. show columns from tbl where field = 'b' or 1=1
. Since we would still be limited to system tables and not expose arbitrary table data, that should be okay as behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just put into backQuoteIfNeed
This is an automated comment for commit f7e31a9 with description of existing statuses. It's updated for the latest CI running
|
primary_key AS expression | ||
FROM system.tables | ||
WHERE | ||
database = '{0}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SQL injection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test with the database name and table name containing the '
character.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parsers of SHOW INDEX
and SHOW COLUMNS
expect database and table as identifiers, whereas WHERE table/database =
expects a string literal. The mismatch is now properly resolved using quoting. I have also added a test.
Ok. There are also manipulators |
Sure, I just found it more readable to write the generated SELECT query using with as little abstractions as possible. |
Ok. While the safest way is AST construction. |
Test failures also occur in master --> merging |
Fixes #49140
The implementation is modeled after MySQL's SHOW INDEX. This leads to some weird naming and a few unused fields ...
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Support statement
SHOW INDEX
to improve compatibility with MySQL