diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index 5c72e37fc3..0d564cc7f8 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -228,6 +228,7 @@ func (s *Service) parseJWTToken(c *gin.Context) { queryStr, err := utils.ParseJWTString("conprof", token) if err != nil { rest.Error(c, rest.ErrBadRequest.WrapWithNoMessage(err)) + c.Abort() return } c.Request.URL.RawQuery = queryStr diff --git a/pkg/apiserver/resource_manager/service.go b/pkg/apiserver/resource_manager/service.go index 8d0e78a4a4..45479dc1fc 100644 --- a/pkg/apiserver/resource_manager/service.go +++ b/pkg/apiserver/resource_manager/service.go @@ -3,8 +3,10 @@ package resourcemanager import ( + "errors" "fmt" "net/http" + "regexp" "time" "github.com/gin-gonic/gin" @@ -17,6 +19,8 @@ import ( "github.com/pingcap/tidb-dashboard/util/rest" ) +var workloadInjectChecker = regexp.MustCompile(`^[a-zA-Z0-9_]+$`) + type ServiceParams struct { fx.In TiDBClient *tidb.Client @@ -109,6 +113,10 @@ func (s *Service) GetCalibrateByHardware(c *gin.Context) { rest.Error(c, rest.ErrBadRequest.New("workload cannot be empty")) return } + if !workloadInjectChecker.MatchString(w) { + rest.Error(c, errors.New("invalid workload")) + return + } db := utils.GetTiDBConnection(c) resp := &CalibrateResponse{} diff --git a/pkg/apiserver/statement/queries.go b/pkg/apiserver/statement/queries.go index efdbf40b83..80fb14e93a 100644 --- a/pkg/apiserver/statement/queries.go +++ b/pkg/apiserver/statement/queries.go @@ -16,7 +16,7 @@ const ( statementsTable = "INFORMATION_SCHEMA.CLUSTER_STATEMENTS_SUMMARY_HISTORY" ) -var injectChecker = regexp.MustCompile(`\s`) +var digestInjectChecker = regexp.MustCompile(`^[a-zA-Z0-9]+$`) func queryStmtTypes(db *gorm.DB) (result []string, err error) { // why should put DISTINCT inside the `Pluck()` method, see here: @@ -210,7 +210,7 @@ func (s *Service) createPlanBinding(db *gorm.DB, planDigest string) (err error) // Caution! SQL injection vulnerability! // We have to interpolate sql string here, since plan binding stmt does not support session level prepare. // go-sql-driver can enable interpolation globally. Refer to https://github.com/go-sql-driver/mysql#interpolateparams. - if injectChecker.MatchString(planDigest) { + if !digestInjectChecker.MatchString(planDigest) { return errors.New("invalid planDigest") } diff --git a/release-version b/release-version index 72f49c8c75..5b9134d514 100644 --- a/release-version +++ b/release-version @@ -1,3 +1,3 @@ # This file specifies the TiDB Dashboard internal version, which will be printed in `--version` # and UI. In release branch, changing this file will result in publishing a new version and tag. -2023.09.11.1 +2023.09.21.1 diff --git a/util/rest/context_helpers.go b/util/rest/context_helpers.go index f0a52ca756..146a0dce80 100644 --- a/util/rest/context_helpers.go +++ b/util/rest/context_helpers.go @@ -6,7 +6,6 @@ import ( "net/http" "github.com/gin-gonic/gin" - "github.com/joomcode/errorx" "github.com/pingcap/tidb-dashboard/util/jsonserde/ginadapter" ) @@ -16,7 +15,8 @@ import ( // Otherwise there will be no error message written to the client. // See `ErrorHandlerFn` for more details. func Error(c *gin.Context, err error) { - _ = c.Error(errorx.EnsureStackTrace(err)) + // For security reasons, we need to hide detailed stacktrace info. + _ = c.Error(err) // before: c.Error(errorx.EnsureStackTrace(err)) } // JSON writes a JSON string to the client with the given status code. diff --git a/util/rest/error_resp.go b/util/rest/error_resp.go index 63513ef380..e5d62c5920 100644 --- a/util/rest/error_resp.go +++ b/util/rest/error_resp.go @@ -95,9 +95,10 @@ func buildDetailMessage(err error) string { func NewErrorResponse(err error) ErrorResponse { return ErrorResponse{ - Error: true, - Message: buildSimpleMessage(err), - Code: removeErrorPrefix(buildCode(err)), - FullText: buildDetailMessage(err), + Error: true, + Message: buildSimpleMessage(err), + Code: removeErrorPrefix(buildCode(err)), + // For security reasons, we need to hide detailed stacktrace info. + // FullText: buildDetailMessage(err), } }