CVE-2021-43798: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Grafana is an open-source platform for monitoring and observability. Grafana versions 8.0.0-beta1 through 8.3.0 (except for patched versions) iss vulnerable to directory traversal, allowing access to local files. The vulnerable URL path is: `<grafana_host_url>/public/plugins//`, where is the plugin ID for any installed plugin. At no time has Grafana Cloud been vulnerable. Users are advised to upgrade to patched versions 8.0.7, 8.1.8, 8.2.7, or 8.3.1. The GitHub Security Advisory contains more information about vulnerable URL paths, mitigation, and the disclosure timeline.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.5
Published: 2021-12-07T19:15Z
Repository: https://github.com/grafana/grafana
Language: Go
Files changed: pkg/api/plugins.go, pkg/api/plugins_test.go
Commit date: 2021-12-07 19:15:53 +0200
Commit message: Security: Fix directory traversal issue (#42846)

* security: fix dir traversal issue

(cherry picked from commit 00e38ba555cfb120361c9623de3285d70c60172f)

* Improve comments and error message.

Co-authored-by: Kyle Brandt <kyle@grafana.com>
Diff: diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go
index 6e01a3414f..ca076671e3 100644
--- a/pkg/api/plugins.go
+++ b/pkg/api/plugins.go
@@ -287,18 +287,31 @@ func (hs *HTTPServer) getPluginAssets(c *models.ReqContext) {
 	if !exists {
 		c.JsonApiErr(404, "Plugin not found", nil)
 		return
 	}
 
-	requestedFile := filepath.Clean(web.Params(c.Req)["*"])
-	pluginFilePath := filepath.Join(plugin.PluginDir, requestedFile)
+	// prepend slash for cleaning relative paths
+	requestedFile := filepath.Clean(filepath.Join("/", web.Params(c.Req)["*"]))
+	rel, err := filepath.Rel("/", requestedFile)
+	if err != nil {
+		// slash is prepended above therefore this is not expected to fail
+		c.JsonApiErr(500, "Failed to get the relative path", err)
+		return
+	}
 
-	if !plugin.IncludedInSignature(requestedFile) {
+	if !plugin.IncludedInSignature(rel) {
 		hs.log.Warn("Access to requested plugin file will be forbidden in upcoming Grafana versions as the file "+
 			"is not included in the plugin signature", "file", requestedFile)
 	}
 
+	absPluginDir, err := filepath.Abs(plugin.PluginDir)
+	if err != nil {
+		c.JsonApiErr(500, "Failed to get plugin absolute path", nil)
+		return
+	}
+
+	pluginFilePath := filepath.Join(absPluginDir, rel)
 	// It's safe to ignore gosec warning G304 since we already clean the requested file path and subsequently
 	// use this with a prefix of the plugin's directory, which is set during plugin loading
 	// nolint:gosec
 	f, err := os.Open(pluginFilePath)
 	if err != nil {
diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go
index 8b6689a9fb..fb10aa0e4d 100644
--- a/pkg/api/plugins_test.go
+++ b/pkg/api/plugins_test.go
@@ -21,13 +21,17 @@ import (
 func Test_GetPluginAssets(t *testing.T) {
 	pluginID := "test-plugin"
 	pluginDir := "."
 	tmpFile, err := ioutil.TempFile(pluginDir, "")
 	require.NoError(t, err)
+	tmpFileInParentDir, err := ioutil.TempFile("..", "")
+	require.NoError(t, err)
 	t.Cleanup(func() {
 		err := os.RemoveAll(tmpFile.Name())
 		assert.NoError(t, err)
+		err = os.RemoveAll(tmpFileInParentDir.Name())
+		assert.NoError(t, err)
 	})
 	expectedBody := "Plugin test"
 	_, err = tmpFile.WriteString(expectedBody)
 	assert.NoError(t, err)
 
@@ -59,10 +63,33 @@ func Test_GetPluginAssets(t *testing.T) {
 				assert.Equal(t, expectedBody, sc.resp.Body.String())
 				assert.Empty(t, l.warnings)
 			})
 	})
 
+	t.Run("Given a request for a relative path", func(t *testing.T) {
+		p := plugins.PluginDTO{
+			JSONData: plugins.JSONData{
+				ID: pluginID,
+			},
+			PluginDir: pluginDir,
+		}
+		service := &fakePluginStore{
+			plugins: map[string]plugins.PluginDTO{
+				pluginID: p,
+			},
+		}
+		l := &logger{}
+
+		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, tmpFileInParentDir.Name())
+		pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l,
+			func(sc *scenarioContext) {
+				callGetPluginAsset(sc)
+
+				require.Equal(t, 404, sc.resp.Code)
+			})
+	})
+
 	t.Run("Given a request for an existing plugin file that is not listed as a signature covered file", func(t *testing.T) {
 		p := plugins.PluginDTO{
 			JSONData: plugins.JSONData{
 				ID: pluginID,
 			},

Vulnerable code: 	requestedFile := filepath.Clean(web.Params(c.Req)["*"])
	pluginFilePath := filepath.Join(plugin.PluginDir, requestedFile)
	if !plugin.IncludedInSignature(requestedFile) {
Fixed code: 	// prepend slash for cleaning relative paths
	requestedFile := filepath.Clean(filepath.Join("/", web.Params(c.Req)["*"]))
	rel, err := filepath.Rel("/", requestedFile)
	if err != nil {
		// slash is prepended above therefore this is not expected to fail
		c.JsonApiErr(500, "Failed to get the relative path", err)
		return
	}
	if !plugin.IncludedInSignature(rel) {
	absPluginDir, err := filepath.Abs(plugin.PluginDir)
	if err != nil {
		c.JsonApiErr(500, "Failed to get plugin absolute path", nil)
		return
	}

	pluginFilePath := filepath.Join(absPluginDir, rel)
	tmpFileInParentDir, err := ioutil.TempFile("..", "")
	require.NoError(t, err)
		err = os.RemoveAll(tmpFileInParentDir.Name())
		assert.NoError(t, err)
	t.Run("Given a request for a relative path", func(t *testing.T) {
		p := plugins.PluginDTO{
			JSONData: plugins.JSONData{
				ID: pluginID,
			},
			PluginDir: pluginDir,
		}
		service := &fakePluginStore{
			plugins: map[string]plugins.PluginDTO{
				pluginID: p,
			},
		}
		l := &logger{}

		url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, tmpFileInParentDir.Name())
		pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l,
			func(sc *scenarioContext) {
				callGetPluginAsset(sc)

				require.Equal(t, 404, sc.resp.Code)
			})
	})