Skip to content

server, sessionctx: add multi statement workaround (#22351)#22469

Closed
ti-srebot wants to merge 1 commit intopingcap:release-5.0-rcfrom
ti-srebot:release-5.0-rc-57eef1333bf1
Closed

server, sessionctx: add multi statement workaround (#22351)#22469
ti-srebot wants to merge 1 commit intopingcap:release-5.0-rcfrom
ti-srebot:release-5.0-rc-57eef1333bf1

Conversation

@ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Jan 21, 2021

cherry-pick #22351 to release-5.0-rc
You can switch your code base to this Pull Request by using git-extras:

# In tidb repo:
git pr https://github.com/pingcap/tidb/pull/22469

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/tidb.git pr/22469:release-5.0-rc-57eef1333bf1

What problem does this PR solve?

Problem Summary:

v4.0.9 shipped with a fix for a server protocol vulnerability: #19459

It can be worked around by changing client library settings, but that's not always easy given each client library is different. This provides a server workaround as well, which adjusts from an error to a warning by default.

What is changed and how it works?

A new sysvar is added, called tidb_multi_statement_mode (scope: SESSION or GLOBAL). The type is an ENUM:

  • OFF: the MySQL compatible/safest behavior. Multi-statement is not permitted unless the client sets the multi-statement attribute. An error is returned.
  • ON: Multi-statement is permitted without errors or warnings.
  • WARN (default): Multi-statement is permitted, but returns a warning.

Thus, the "4.0.8 and earlier" behavior can be restored with "ON". The 4.0.9 and 4.0.10 behavior is effectively "OFF".

Both the warning and error message is as follows:

client has multi-statement capability disabled. Run SET GLOBAL tidb_multi_statement_mode='ON' after you understand the security risk

The intention is to change the default from WARN back to OFF in a 4.0-series release in the short future, so users are safe-by-default. In order to do this, SQL client error tracking will have to be added (see #14433 ). This PR ensures that this error uses the unique code of 8030 so that deployment tools can check if a user depends on the unsafe behavior before attempting to upgrade them.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

  • Introduces security risk again (but not by default, and not materially different from enabling client multi-statement)

Release note

  • TiDB 4.0.9 fixed a security issue, where TiDB incorrectly permitted multiple statements to be executed in one COM_QUERY packet, leading to increased risk of SQL injection. To provide backwards compatibility for applications that depend on this behavior, a new option tidb_multi_statement_mode has been added. Assuming you understand the security risks, you can revert to the 4.0.8 by executing SET GLOBAL tidb_multi_statement_mode='ON'. The default behavior of tidb_multi_statement_mode also relaxes the error introduced in 4.0.9 to a warning. It is intended to be changed to an error again in a future release.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@morgo please accept the invitation then you can push to the cherry-pick pull requests.
https://github.com/ti-srebot/tidb/invitations

@morgo
Copy link
Contributor

morgo commented Jan 21, 2021

I am going to close this because it doesn't apply cleanly. The behavior will be picked up as 5.0 rebranches from master.

@morgo morgo closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/sql-infra SIG: SQL Infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants