Release v2023.09.21.01 [skip pd_pr] (#1596)

* security fixes (#1592) * hide full stacktrace info in error message (#1593) * update release-version to 2023.09.21.1 Signed-off-by: mornyx <[email protected]> --------- Signed-off-by: mornyx <[email protected]>
pingcap · Sep 21, 2023 · 31ea86f · 31ea86f
1 parent 22add1e
commit 31ea86f
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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),
 	}
 }