Skip to content

Commit

Permalink
MySQL: Limit datasource error details returned from the backend (graf…
Browse files Browse the repository at this point in the history 
…ana#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 grafana#19360
  • Loading branch information
@marefr
marefr authored Sep 24, 2019
1 parent 7c499ff commit 3de693a
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 32 deletions.
12 changes: 8 additions & 4 deletions pkg/tsdb/mssql/mssql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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))

Expand Down Expand Up @@ -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
}
16 changes: 14 additions & 2 deletions pkg/tsdb/mysql/macros.go
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,43 @@
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"
)

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

Expand Down
33 changes: 32 additions & 1 deletion pkg/tsdb/mysql/macros_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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")
}
})
})
}
24 changes: 20 additions & 4 deletions pkg/tsdb/mysql/mysql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
12 changes: 8 additions & 4 deletions pkg/tsdb/postgres/postgres.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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))

Expand Down Expand Up @@ -93,3 +93,7 @@ func (t *postgresRowTransformer) Transform(columnTypes []*sql.ColumnType, rows *

return values, nil
}

func (t *postgresQueryResultTransformer) TransformQueryError(err error) error {
return err
}
37 changes: 20 additions & 17 deletions pkg/tsdb/sqleng/sql_engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 3de693a

Please sign in to comment.