From 3de693af4981c1c69aa897eb4b0717cef9f12836 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 24 Sep 2019 20:50:49 +0200 Subject: [PATCH] MySQL: Limit datasource error details returned from the backend (#19373) Only return certain mysql errors from backend. The following errors is returned as is from backend: error code 1064 (parse error) error code 1054 (bad column/field selected) error code 1146 (table not exists) Any other errors is logged and returned as a generic error. Restrict use of certain functions: Do not allow usage of the following in query: system_user() session_user() current_user() or current_user user() show grants Fixes #19360 --- pkg/tsdb/mssql/mssql.go | 12 ++++++++---- pkg/tsdb/mysql/macros.go | 16 +++++++++++++-- pkg/tsdb/mysql/macros_test.go | 33 ++++++++++++++++++++++++++++++- pkg/tsdb/mysql/mysql.go | 24 +++++++++++++++++++---- pkg/tsdb/postgres/postgres.go | 12 ++++++++---- pkg/tsdb/sqleng/sql_engine.go | 37 +++++++++++++++++++---------------- 6 files changed, 102 insertions(+), 32 deletions(-) diff --git a/pkg/tsdb/mssql/mssql.go b/pkg/tsdb/mssql/mssql.go index 8f6176c41f3f7..21884431156f1 100644 --- a/pkg/tsdb/mssql/mssql.go +++ b/pkg/tsdb/mssql/mssql.go @@ -38,11 +38,11 @@ func newMssqlQueryEndpoint(datasource *models.DataSource) (tsdb.TsdbQueryEndpoin MetricColumnTypes: []string{"VARCHAR", "CHAR", "NVARCHAR", "NCHAR"}, } - rowTransformer := mssqlRowTransformer{ + queryResultTransformer := mssqlQueryResultTransformer{ log: logger, } - return sqleng.NewSqlQueryEndpoint(&config, &rowTransformer, newMssqlMacroEngine(), logger) + return sqleng.NewSqlQueryEndpoint(&config, &queryResultTransformer, newMssqlMacroEngine(), logger) } func generateConnectionString(datasource *models.DataSource) (string, error) { @@ -62,11 +62,11 @@ func generateConnectionString(datasource *models.DataSource) (string, error) { return connStr, nil } -type mssqlRowTransformer struct { +type mssqlQueryResultTransformer struct { log log.Logger } -func (t *mssqlRowTransformer) Transform(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) { +func (t *mssqlQueryResultTransformer) TransformQueryResult(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) { values := make([]interface{}, len(columnTypes)) valuePtrs := make([]interface{}, len(columnTypes)) @@ -100,3 +100,7 @@ func (t *mssqlRowTransformer) Transform(columnTypes []*sql.ColumnType, rows *cor return values, nil } + +func (t *mssqlQueryResultTransformer) TransformQueryError(err error) error { + return err +} diff --git a/pkg/tsdb/mysql/macros.go b/pkg/tsdb/mysql/macros.go index b10d66bab49e2..1fb924ecfbdf0 100644 --- a/pkg/tsdb/mysql/macros.go +++ b/pkg/tsdb/mysql/macros.go @@ -1,11 +1,13 @@ package mysql import ( + "errors" "fmt" "regexp" "strings" "github.com/grafana/grafana/pkg/components/gtime" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/tsdb" "github.com/grafana/grafana/pkg/tsdb/sqleng" ) @@ -13,19 +15,29 @@ import ( const rsIdentifier = `([_a-zA-Z0-9]+)` const sExpr = `\$` + rsIdentifier + `\(([^\)]*)\)` +var restrictedRegExp = regexp.MustCompile(`(?im)([\s]*show[\s]+grants|[\s,]session_user\([^\)]*\)|[\s,]current_user(\([^\)]*\))?|[\s,]system_user\([^\)]*\)|[\s,]user\([^\)]*\))([\s,;]|$)`) + type mySqlMacroEngine struct { *sqleng.SqlMacroEngineBase timeRange *tsdb.TimeRange query *tsdb.Query + logger log.Logger } -func newMysqlMacroEngine() sqleng.SqlMacroEngine { - return &mySqlMacroEngine{SqlMacroEngineBase: sqleng.NewSqlMacroEngineBase()} +func newMysqlMacroEngine(logger log.Logger) sqleng.SqlMacroEngine { + return &mySqlMacroEngine{SqlMacroEngineBase: sqleng.NewSqlMacroEngineBase(), logger: logger} } func (m *mySqlMacroEngine) Interpolate(query *tsdb.Query, timeRange *tsdb.TimeRange, sql string) (string, error) { m.timeRange = timeRange m.query = query + + matches := restrictedRegExp.FindAllStringSubmatch(sql, 1) + if len(matches) > 0 { + m.logger.Error("show grants, session_user(), current_user(), system_user() or user() not allowed in query") + return "", errors.New("Invalid query. Inspect Grafana server log for details") + } + rExp, _ := regexp.Compile(sExpr) var macroError error diff --git a/pkg/tsdb/mysql/macros_test.go b/pkg/tsdb/mysql/macros_test.go index 7d129e845b13d..b00935ed4ddca 100644 --- a/pkg/tsdb/mysql/macros_test.go +++ b/pkg/tsdb/mysql/macros_test.go @@ -6,13 +6,16 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/tsdb" . "github.com/smartystreets/goconvey/convey" ) func TestMacroEngine(t *testing.T) { Convey("MacroEngine", t, func() { - engine := &mySqlMacroEngine{} + engine := &mySqlMacroEngine{ + logger: log.New("test"), + } query := &tsdb.Query{} Convey("Given a time range between 2018-04-12 00:00 and 2018-04-12 00:05", func() { @@ -157,5 +160,33 @@ func TestMacroEngine(t *testing.T) { So(sql, ShouldEqual, fmt.Sprintf("select time >= %d AND time <= %d", from.Unix(), to.Unix())) }) }) + + Convey("Given queries that contains unallowed user functions", func() { + tcs := []string{ + "select \nSESSION_USER(), abc", + "SELECT session_User( ) ", + "SELECT session_User( )\n", + "SELECT current_user", + "SELECT current_USER", + "SELECT current_user()", + "SELECT Current_User()", + "SELECT current_user( )", + "SELECT current_user(\t )", + "SELECT user()", + "SELECT USER()", + "SELECT SYSTEM_USER()", + "SELECT System_User()", + "SELECT System_User( )", + "SELECT System_User(\t \t)", + "SHOW \t grants", + " show Grants\n", + "show grants;", + } + + for _, tc := range tcs { + _, err := engine.Interpolate(nil, nil, tc) + So(err.Error(), ShouldEqual, "Invalid query. Inspect Grafana server log for details") + } + }) }) } diff --git a/pkg/tsdb/mysql/mysql.go b/pkg/tsdb/mysql/mysql.go index 1b0c7230c9167..2aa0db170e57c 100644 --- a/pkg/tsdb/mysql/mysql.go +++ b/pkg/tsdb/mysql/mysql.go @@ -2,11 +2,14 @@ package mysql import ( "database/sql" + "errors" "fmt" "reflect" "strconv" "strings" + "github.com/VividCortex/mysqlerr" + "github.com/grafana/grafana/pkg/setting" "github.com/go-sql-driver/mysql" @@ -59,18 +62,18 @@ func newMysqlQueryEndpoint(datasource *models.DataSource) (tsdb.TsdbQueryEndpoin MetricColumnTypes: []string{"CHAR", "VARCHAR", "TINYTEXT", "TEXT", "MEDIUMTEXT", "LONGTEXT"}, } - rowTransformer := mysqlRowTransformer{ + rowTransformer := mysqlQueryResultTransformer{ log: logger, } - return sqleng.NewSqlQueryEndpoint(&config, &rowTransformer, newMysqlMacroEngine(), logger) + return sqleng.NewSqlQueryEndpoint(&config, &rowTransformer, newMysqlMacroEngine(logger), logger) } -type mysqlRowTransformer struct { +type mysqlQueryResultTransformer struct { log log.Logger } -func (t *mysqlRowTransformer) Transform(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) { +func (t *mysqlQueryResultTransformer) TransformQueryResult(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) { values := make([]interface{}, len(columnTypes)) for i := range values { @@ -128,3 +131,16 @@ func (t *mysqlRowTransformer) Transform(columnTypes []*sql.ColumnType, rows *cor return values, nil } + +func (t *mysqlQueryResultTransformer) TransformQueryError(err error) error { + if driverErr, ok := err.(*mysql.MySQLError); ok { + if driverErr.Number != mysqlerr.ER_PARSE_ERROR && driverErr.Number != mysqlerr.ER_BAD_FIELD_ERROR && driverErr.Number != mysqlerr.ER_NO_SUCH_TABLE { + t.log.Error("query error", "err", err) + return errQueryFailed + } + } + + return err +} + +var errQueryFailed = errors.New("Query failed. Please inspect Grafana server log for details") diff --git a/pkg/tsdb/postgres/postgres.go b/pkg/tsdb/postgres/postgres.go index 9836cf16a1961..a417699c1d162 100644 --- a/pkg/tsdb/postgres/postgres.go +++ b/pkg/tsdb/postgres/postgres.go @@ -33,13 +33,13 @@ func newPostgresQueryEndpoint(datasource *models.DataSource) (tsdb.TsdbQueryEndp MetricColumnTypes: []string{"UNKNOWN", "TEXT", "VARCHAR", "CHAR"}, } - rowTransformer := postgresRowTransformer{ + queryResultTransformer := postgresQueryResultTransformer{ log: logger, } timescaledb := datasource.JsonData.Get("timescaledb").MustBool(false) - return sqleng.NewSqlQueryEndpoint(&config, &rowTransformer, newPostgresMacroEngine(timescaledb), logger) + return sqleng.NewSqlQueryEndpoint(&config, &queryResultTransformer, newPostgresMacroEngine(timescaledb), logger) } func generateConnectionString(datasource *models.DataSource) string { @@ -54,11 +54,11 @@ func generateConnectionString(datasource *models.DataSource) string { return u.String() } -type postgresRowTransformer struct { +type postgresQueryResultTransformer struct { log log.Logger } -func (t *postgresRowTransformer) Transform(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) { +func (t *postgresQueryResultTransformer) TransformQueryResult(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) { values := make([]interface{}, len(columnTypes)) valuePtrs := make([]interface{}, len(columnTypes)) @@ -93,3 +93,7 @@ func (t *postgresRowTransformer) Transform(columnTypes []*sql.ColumnType, rows * return values, nil } + +func (t *postgresQueryResultTransformer) TransformQueryError(err error) error { + return err +} diff --git a/pkg/tsdb/sqleng/sql_engine.go b/pkg/tsdb/sqleng/sql_engine.go index 3806c392344d1..9b3f17fdf786c 100644 --- a/pkg/tsdb/sqleng/sql_engine.go +++ b/pkg/tsdb/sqleng/sql_engine.go @@ -31,9 +31,12 @@ type SqlMacroEngine interface { Interpolate(query *tsdb.Query, timeRange *tsdb.TimeRange, sql string) (string, error) } -// SqlTableRowTransformer transforms a query result row to RowValues with proper types. -type SqlTableRowTransformer interface { - Transform(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) +// SqlQueryResultTransformer transforms a query result row to RowValues with proper types. +type SqlQueryResultTransformer interface { + // TransformQueryResult transforms a query result row to RowValues with proper types. + TransformQueryResult(columnTypes []*sql.ColumnType, rows *core.Rows) (tsdb.RowValues, error) + // TransformQueryError transforms a query error. + TransformQueryError(err error) error } type engineCacheType struct { @@ -54,12 +57,12 @@ var NewXormEngine = func(driverName string, connectionString string) (*xorm.Engi } type sqlQueryEndpoint struct { - macroEngine SqlMacroEngine - rowTransformer SqlTableRowTransformer - engine *xorm.Engine - timeColumnNames []string - metricColumnTypes []string - log log.Logger + macroEngine SqlMacroEngine + queryResultTransformer SqlQueryResultTransformer + engine *xorm.Engine + timeColumnNames []string + metricColumnTypes []string + log log.Logger } type SqlQueryEndpointConfiguration struct { @@ -70,12 +73,12 @@ type SqlQueryEndpointConfiguration struct { MetricColumnTypes []string } -var NewSqlQueryEndpoint = func(config *SqlQueryEndpointConfiguration, rowTransformer SqlTableRowTransformer, macroEngine SqlMacroEngine, log log.Logger) (tsdb.TsdbQueryEndpoint, error) { +var NewSqlQueryEndpoint = func(config *SqlQueryEndpointConfiguration, queryResultTransformer SqlQueryResultTransformer, macroEngine SqlMacroEngine, log log.Logger) (tsdb.TsdbQueryEndpoint, error) { queryEndpoint := sqlQueryEndpoint{ - rowTransformer: rowTransformer, - macroEngine: macroEngine, - timeColumnNames: []string{"time"}, - log: log, + queryResultTransformer: queryResultTransformer, + macroEngine: macroEngine, + timeColumnNames: []string{"time"}, + log: log, } if len(config.TimeColumnNames) > 0 { @@ -160,7 +163,7 @@ func (e *sqlQueryEndpoint) Query(ctx context.Context, dsInfo *models.DataSource, rows, err := db.Query(rawSQL) if err != nil { - queryResult.Error = err + queryResult.Error = e.queryResultTransformer.TransformQueryError(err) return } @@ -242,7 +245,7 @@ func (e *sqlQueryEndpoint) transformToTable(query *tsdb.Query, rows *core.Rows, return fmt.Errorf("query row limit exceeded, limit %d", rowLimit) } - values, err := e.rowTransformer.Transform(columnTypes, rows) + values, err := e.queryResultTransformer.TransformQueryResult(columnTypes, rows) if err != nil { return err } @@ -340,7 +343,7 @@ func (e *sqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core.R return fmt.Errorf("query row limit exceeded, limit %d", rowLimit) } - values, err := e.rowTransformer.Transform(columnTypes, rows) + values, err := e.queryResultTransformer.TransformQueryResult(columnTypes, rows) if err != nil { return err }