Merge pull request #10921 from bergquist/invalid_uid

dashboard: whitelist allowed chars for uid

pkg/services/dashboards/dashboards.go

@@ -6,6 +6,7 @@ import (
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/alerting"
+	"github.com/grafana/grafana/pkg/util"
 )
 
 type Repository interface {
@@ -52,6 +53,10 @@ func (dr *DashboardRepository) buildSaveDashboardCommand(dto *SaveDashboardDTO)
 		return nil, models.ErrDashboardTitleEmpty
 	}
 
+	if err := util.VerifyUid(dashboard.Uid); err != nil {
+		return nil, err
+	}
+
 	validateAlertsCmd := alerting.ValidateDashboardAlertsCommand{
 		OrgId:     dto.OrgId,
 		Dashboard: dashboard,

pkg/services/dashboards/dashboards_test.go

@@ -0,0 +1,43 @@
+package dashboards
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/alerting"
+	"github.com/grafana/grafana/pkg/util"
+)
+
+func TestDashboardsService(t *testing.T) {
+
+	bus.ClearBusHandlers()
+
+	bus.AddHandler("test", func(cmd *alerting.ValidateDashboardAlertsCommand) error {
+		return nil
+	})
+
+	testCases := []struct {
+		Uid   string
+		Error error
+	}{
+		{Uid: "", Error: nil},
+		{Uid: "asdf90_-", Error: nil},
+		{Uid: "asdf/90", Error: util.ErrDashboardInvalidUid},
+		{Uid: "asdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnm", Error: util.ErrDashboardUidToLong},
+	}
+
+	repo := &DashboardRepository{}
+
+	for _, tc := range testCases {
+		dto := &SaveDashboardDTO{
+			Dashboard: &models.Dashboard{Title: "title", Uid: tc.Uid},
+		}
+
+		_, err := repo.buildSaveDashboardCommand(dto)
+
+		if err != tc.Error {
+			t.Fatalf("expected %s to return %v", tc.Uid, tc.Error)
+		}
+	}
+}

pkg/util/shortid_generator.go

@@ -1,11 +1,33 @@
 package util
 
 import (
+	"errors"
+	"regexp"
+
 	"github.com/teris-io/shortid"
 )
 
+var allowedChars = shortid.DefaultABC
+
+var validUidPattern = regexp.MustCompile(`^[a-zA-Z0-9\-\_]*$`).MatchString
+
+var ErrDashboardInvalidUid = errors.New("uid contains illegal characters")
+var ErrDashboardUidToLong = errors.New("uid to long. max 40 characters")
+
+func VerifyUid(uid string) error {
+	if len(uid) > 40 {
+		return ErrDashboardUidToLong
+	}
+
+	if !validUidPattern(uid) {
+		return ErrDashboardInvalidUid
+	}
+
+	return nil
+}
+
 func init() {
-	gen, _ := shortid.New(1, shortid.DefaultABC, 1)
+	gen, _ := shortid.New(1, allowedChars, 1)
 	shortid.SetDefault(gen)
 }
 

pkg/util/shortid_generator_test.go

@@ -0,0 +1,12 @@
+package util
+
+import "testing"
+
+func TestAllowedCharMatchesUidPattern(t *testing.T) {
+	for _, c := range allowedChars {
+		err := VerifyUid(string(c))
+		if err != nil {
+			t.Fatalf("charset for creating new shortids contains chars not present in uid pattern")
+		}
+	}
+}