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

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

Merged
merged 3 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion errno/errcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,8 @@ const (
ErrBRIERestoreFailed = 8125
ErrBRIEImportFailed = 8126
ErrBRIEExportFailed = 8127
ErrJSONObjectKeyTooLong = 8129
ErrJSONObjectKeyTooLong = 8129 // skip ErrInvalidTableSample (8128) in master only
ErrMultiStatementDisabled = 8130

// Error codes used by TiDB ddl package
ErrUnsupportedDDLOperation = 8200
Expand Down
3 changes: 2 additions & 1 deletion errno/errname.go
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,8 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{
ErrBRIEImportFailed: mysql.Message("Import failed: %s", nil),
ErrBRIEExportFailed: mysql.Message("Export failed: %s", nil),

ErrJSONObjectKeyTooLong: mysql.Message("TiDB does not yet support JSON objects with the key length >= 65536", nil),
ErrJSONObjectKeyTooLong: mysql.Message("TiDB does not yet support JSON objects with the key length >= 65536", nil),
ErrMultiStatementDisabled: mysql.Message("client has multi-statement capability disabled. Run SET GLOBAL tidb_multi_statement_mode='ON' after you understand the security risk", nil),

// TiKV/PD errors.
ErrPDServerTimeout: mysql.Message("PD server timeout", nil),
Expand Down
23 changes: 20 additions & 3 deletions server/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -1384,19 +1384,32 @@ func (cc *clientConn) handleQuery(ctx context.Context, sql string) (err error) {
return err
}

var appendMultiStmtWarning bool

if len(stmts) > 1 {
// The client gets to choose if it allows multi-statements, and
// probably defaults OFF. This helps prevent against SQL injection attacks
// by early terminating the first statement, and then running an entirely
// new statement.
capabilities := cc.ctx.GetSessionVars().ClientCapability
if capabilities&mysql.ClientMultiStatements < 1 {
return errMultiStatementDisabled
// The client does not have multi-statement enabled. We now need to determine
// how to handle an unsafe sitution based on the multiStmt sysvar.
switch cc.ctx.GetSessionVars().MultiStatementMode {
case variable.OffInt:
err = errMultiStatementDisabled
metrics.ExecuteErrorCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err)).Inc()
return err
case variable.OnInt:
// multi statement is fully permitted, do nothing
default:
appendMultiStmtWarning = true
}
}
}

for i, stmt := range stmts {
if err = cc.handleStmt(ctx, stmt, i == len(stmts)-1); err != nil {
if err = cc.handleStmt(ctx, stmt, i == len(stmts)-1, appendMultiStmtWarning); err != nil {
break
}
}
Expand All @@ -1406,7 +1419,7 @@ func (cc *clientConn) handleQuery(ctx context.Context, sql string) (err error) {
return err
}

func (cc *clientConn) handleStmt(ctx context.Context, stmt ast.StmtNode, lastStmt bool) error {
func (cc *clientConn) handleStmt(ctx context.Context, stmt ast.StmtNode, lastStmt bool, appendMultiStmtWarning bool) error {
rs, err := cc.ctx.ExecuteStmt(ctx, stmt)
if rs != nil {
defer terror.Call(rs.Close)
Expand All @@ -1415,6 +1428,10 @@ func (cc *clientConn) handleStmt(ctx context.Context, stmt ast.StmtNode, lastStm
return err
}

if lastStmt && appendMultiStmtWarning {
cc.ctx.GetSessionVars().StmtCtx.AppendWarning(errMultiStatementDisabled)
}

status := cc.ctx.Status()
if !lastStmt {
status |= mysql.ServerMoreResultsExists
Expand Down
2 changes: 1 addition & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ var (
errAccessDenied = dbterror.ClassServer.NewStd(errno.ErrAccessDenied)
errConCount = dbterror.ClassServer.NewStd(errno.ErrConCount)
errSecureTransportRequired = dbterror.ClassServer.NewStd(errno.ErrSecureTransportRequired)
errMultiStatementDisabled = dbterror.ClassServer.NewStdErr(errno.ErrUnknown, mysql.Message("client has multi-statement capability disabled", nil)) // MySQL returns a parse error
errMultiStatementDisabled = dbterror.ClassServer.NewStd(errno.ErrMultiStatementDisabled)
)

// DefaultCapability is the capability of the server when it is created using the default configuration.
Expand Down
64 changes: 62 additions & 2 deletions server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1179,8 +1179,68 @@ func (cli *testServerClient) runTestStatusAPI(c *C) {

func (cli *testServerClient) runFailedTestMultiStatements(c *C) {
cli.runTestsOnNewDB(c, nil, "FailedMultiStatements", func(dbt *DBTest) {
_, err := dbt.db.Exec("SELECT 1; SELECT 1; SELECT 2; SELECT 3;")
c.Assert(err.Error(), Equals, "Error 1105: client has multi-statement capability disabled")

// Default of WARN
dbt.mustExec("CREATE TABLE `test` (`id` int(11) NOT NULL, `value` int(11) NOT NULL) ")
res := dbt.mustExec("INSERT INTO test VALUES (1, 1)")
count, err := res.RowsAffected()
c.Assert(err, IsNil, Commentf("res.RowsAffected() returned error"))
c.Assert(count, Equals, int64(1))
res = dbt.mustExec("UPDATE test SET value = 3 WHERE id = 1; UPDATE test SET value = 4 WHERE id = 1; UPDATE test SET value = 5 WHERE id = 1;")
count, err = res.RowsAffected()
c.Assert(err, IsNil, Commentf("res.RowsAffected() returned error"))
c.Assert(count, Equals, int64(1))
rows := dbt.mustQuery("show warnings")
c.Assert(rows.Next(), IsTrue)
var level, code, message string
err = rows.Scan(&level, &code, &message)
c.Assert(err, IsNil)
c.Assert(rows.Close(), IsNil)
c.Assert(level, Equals, "Warning")
c.Assert(code, Equals, "8130")
c.Assert(message, Equals, "client has multi-statement capability disabled. Run SET GLOBAL tidb_multi_statement_mode='ON' after you understand the security risk")
var out int
rows = dbt.mustQuery("SELECT value FROM test WHERE id=1;")
if rows.Next() {
rows.Scan(&out)
c.Assert(out, Equals, 5)

if rows.Next() {
dbt.Error("unexpected data")
}
} else {
dbt.Error("no data")
}

// Change to OFF = Does not work
dbt.mustExec("SET tidb_multi_statement_mode='OFF'")
_, err = dbt.db.Exec("SELECT 1; SELECT 1; SELECT 2; SELECT 3;")
c.Assert(err.Error(), Equals, "Error 8130: client has multi-statement capability disabled. Run SET GLOBAL tidb_multi_statement_mode='ON' after you understand the security risk")

// Change to ON = Fully supported, TiDB legacy. No warnings or Errors.
dbt.mustExec("SET tidb_multi_statement_mode='ON';")
dbt.mustExec("DROP TABLE IF EXISTS test")
dbt.mustExec("CREATE TABLE `test` (`id` int(11) NOT NULL, `value` int(11) NOT NULL) ")
res = dbt.mustExec("INSERT INTO test VALUES (1, 1)")
count, err = res.RowsAffected()
c.Assert(err, IsNil, Commentf("res.RowsAffected() returned error"))
c.Assert(count, Equals, int64(1))
res = dbt.mustExec("update test SET value = 3 WHERE id = 1; UPDATE test SET value = 4 WHERE id = 1; UPDATE test SET value = 5 WHERE id = 1;")
count, err = res.RowsAffected()
c.Assert(err, IsNil, Commentf("res.RowsAffected() returned error"))
c.Assert(count, Equals, int64(1))
rows = dbt.mustQuery("SELECT value FROM test WHERE id=1;")
if rows.Next() {
rows.Scan(&out)
c.Assert(out, Equals, 5)

if rows.Next() {
dbt.Error("unexpected data")
}
} else {
dbt.Error("no data")
}

})
}

Expand Down
1 change: 1 addition & 0 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -2115,6 +2115,7 @@ var builtinGlobalVariable = []string{
variable.TiDBEnableAmendPessimisticTxn,
variable.TiDBEnableRateLimitAction,
variable.TiDBMemoryUsageAlarmRatio,
variable.TiDBMultiStatementMode,
}

var (
Expand Down
5 changes: 5 additions & 0 deletions sessionctx/variable/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,9 @@ type SessionVars struct {
// AllowDistinctAggPushDown can be set true to allow agg with distinct push down to tikv/tiflash.
AllowDistinctAggPushDown bool

// MultiStatementMode permits incorrect client library usage. Not recommended to be turned on.
MultiStatementMode int

// AllowWriteRowID can be set to false to forbid write data to _tidb_rowid.
// This variable is currently not recommended to be turned on.
AllowWriteRowID bool
Expand Down Expand Up @@ -1390,6 +1393,8 @@ func (s *SessionVars) SetSystemVar(name string, val string) error {
return ErrWrongValueForVar.GenWithStackByArgs(TiDBMemoryUsageAlarmRatio, val)
}
MemoryUsageAlarmRatio.Store(floatVal)
case TiDBMultiStatementMode:
s.MultiStatementMode = TiDBOptMultiStmt(val)
}
s.systems[name] = val
return nil
Expand Down
8 changes: 8 additions & 0 deletions sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ const (
ScopeGlobal ScopeFlag = 1 << 0
// ScopeSession means the system variable can only be changed in current session.
ScopeSession ScopeFlag = 1 << 1

// Off is the string OFF
Off = "OFF"
// On is the string ON
On = "ON"
// Warn is the string WARN
Warn = "WARN"
)

// SysVar is for system variable.
Expand Down Expand Up @@ -733,6 +740,7 @@ var defaultSysVars = []*SysVar{
{ScopeGlobal | ScopeSession, TiDBRedactLog, BoolToIntStr(DefTiDBRedactLog)},
{ScopeGlobal, TiDBEnableTelemetry, BoolToIntStr(DefTiDBEnableTelemetry)},
{ScopeGlobal | ScopeSession, TiDBEnableAmendPessimisticTxn, boolToOnOff(DefTiDBEnableAmendPessimisticTxn)},
{ScopeGlobal | ScopeSession, TiDBMultiStatementMode, Warn},
}

// SynonymsSysVariables is synonyms of system variables.
Expand Down
4 changes: 4 additions & 0 deletions sessionctx/variable/tidb_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ const (
// TiDBAllowRemoveAutoInc indicates whether a user can drop the auto_increment column attribute or not.
TiDBAllowRemoveAutoInc = "tidb_allow_remove_auto_inc"

// TiDBMultiStatementMode enables multi statement at the risk of SQL injection
// provides backwards compatibility
TiDBMultiStatementMode = "tidb_multi_statement_mode"

// TiDBEvolvePlanTaskMaxTime controls the max time of a single evolution task.
TiDBEvolvePlanTaskMaxTime = "tidb_evolve_plan_task_max_time"

Expand Down
43 changes: 42 additions & 1 deletion sessionctx/variable/varsutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -809,13 +809,54 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string, scope Sc
if _, err := collate.GetCollationByName(value); err != nil {
return value, errors.Trace(err)
}
case TiDBMultiStatementMode:
if TiDBOptOn(value) {
return On, nil
}
if TiDBOptOff(value) {
return Off, nil
}
if TiDBOptWarn(value) {
return Warn, nil
}
return value, ErrWrongValueForVar.GenWithStackByArgs(name, value)
}
return value, nil
}

// TiDBOptOn could be used for all tidb session variable options, we use "ON"/1 to turn on those options.
func TiDBOptOn(opt string) bool {
return strings.EqualFold(opt, "ON") || opt == "1"
return strings.EqualFold(opt, On) || opt == "1"
}

// TiDBOptOff could be used for all tidb session variable options, we use "OFF"/0 to turn on those options.
func TiDBOptOff(opt string) bool {
return strings.EqualFold(opt, Off) || opt == "0"
}

// TiDBOptWarn is used for 3-state booleans
func TiDBOptWarn(opt string) bool {
return strings.EqualFold(opt, Warn) || opt == "2"
}

const (
// OffInt is used by TiDBMultiStatementMode
OffInt = 0
// OnInt is used TiDBMultiStatementMode
OnInt = 1
// WarnInt is used by TiDBMultiStatementMode
WarnInt = 2
)

// TiDBOptMultiStmt converts multi-stmt options to int.
func TiDBOptMultiStmt(opt string) int {
switch opt {
case Off:
return OffInt
case On:
return OnInt
}
return WarnInt
}

func tidbOptPositiveInt32(opt string, defaultVal int) int {
Expand Down