CodeQL: Try to fix uncontrolled data used in path expression (#43462)

Ref #43080

pkg/plugins/manager/dashboards.go

@@ -2,14 +2,17 @@ package manager
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/services/dashboards"
+	"github.com/grafana/grafana/pkg/util"
 )
 
 func (m *PluginManager) GetPluginDashboards(ctx context.Context, orgID int64, pluginID string) ([]*plugins.PluginDashboardInfoDTO, error) {
@@ -73,16 +76,43 @@ func (m *PluginManager) GetPluginDashboards(ctx context.Context, orgID int64, pl
 }
 
 func (m *PluginManager) LoadPluginDashboard(ctx context.Context, pluginID, path string) (*models.Dashboard, error) {
+	if len(strings.TrimSpace(pluginID)) == 0 {
+		return nil, fmt.Errorf("pluginID cannot be empty")
+	}
+
+	if len(strings.TrimSpace(path)) == 0 {
+		return nil, fmt.Errorf("path cannot be empty")
+	}
+
 	plugin, exists := m.Plugin(ctx, pluginID)
 	if !exists {
 		return nil, plugins.NotFoundError{PluginID: pluginID}
 	}
 
-	dashboardFilePath := filepath.Join(plugin.PluginDir, path)
+	cleanPath, err := util.CleanRelativePath(path)
+	if err != nil {
+		// CleanRelativePath should clean and make the path relative so this is not expected to fail
+		return nil, err
+	}
+
+	dashboardFilePath := filepath.Join(plugin.PluginDir, cleanPath)
+
+	included := false
+	for _, include := range plugin.DashboardIncludes() {
+		if filepath.Join(plugin.PluginDir, include.Path) == dashboardFilePath {
+			included = true
+			break
+		}
+	}
+
+	if !included {
+		return nil, fmt.Errorf("dashboard not included in plugin")
+	}
+
 	// nolint:gosec
 	// We can ignore the gosec G304 warning on this one because `plugin.PluginDir` is based
-	// on plugin folder structure on disk and not user input. `path` comes from the
+	// on plugin folder structure on disk and not user input. `path` input validation above
-	// `plugin.json` configuration file for the loaded plugin
+	// should only allow paths defined in the plugin's plugin.json.
 	reader, err := os.Open(dashboardFilePath)
 	if err != nil {
 		return nil, err

pkg/plugins/plugins.go

@@ -144,6 +144,17 @@ type JSONData struct {
 	Executable string `json:"executable,omitempty"`
 }
 
+func (d JSONData) DashboardIncludes() []*Includes {
+	result := []*Includes{}
+	for _, include := range d.Includes {
+		if include.Type == TypeDashboard {
+			result = append(result, include)
+		}
+	}
+
+	return result
+}
+
 // Route describes a plugin route that is defined in
 // the plugin.json file for a plugin.
 type Route struct {

pkg/util/filepath.go

@@ -138,3 +138,19 @@ func containsDistFolder(subFiles []subFile) bool {
 
 	return false
 }
+
+// CleanRelativePath returns the shortest path name equivalent to path
+// by purely lexical processing. It make sure the provided path is rooted
+// and then uses filepath.Clean and filepath.Rel to make sure the path
+// doesn't include any separators or elements that shouldn't be there
+// like ., .., //.
+func CleanRelativePath(path string) (string, error) {
+	cleanPath := filepath.Clean(filepath.Join("/", path))
+	rel, err := filepath.Rel("/", cleanPath)
+	if err != nil {
+		// slash is prepended above therefore this is not expected to fail
+		return "", err
+	}
+
+	return rel, nil
+}

pkg/util/filepath_test.go

@@ -0,0 +1,37 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCleanRelativePath(t *testing.T) {
+	testcases := []struct {
+		input        string
+		expectedPath string
+	}{
+		{
+			input:        "",
+			expectedPath: ".",
+		},
+		{
+			input:        "/test/test.txt",
+			expectedPath: "test/test.txt",
+		},
+		{
+			input:        "../../test/test.txt",
+			expectedPath: "test/test.txt",
+		},
+		{
+			input:        "./../test/test.txt",
+			expectedPath: "test/test.txt",
+		},
+	}
+
+	for _, tt := range testcases {
+		path, err := CleanRelativePath(tt.input)
+		assert.NoError(t, err)
+		assert.Equal(t, tt.expectedPath, path)
+	}
+}