[v8.1.x] Security: Fix directory traversal issue (#42840)

* security: fix dir traversal issue

* Improve comments and error message.

* Disable e2e tests

Co-authored-by: Kyle Brandt <kyle@grafana.com>
Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>
This commit is contained in:
Dimitris Sotirakis
2021-12-07 18:42:12 +01:00
committed by GitHub
co-authored by Kyle Brandt Sofia Papagiannaki
parent 2b8ce2ab34
commit 85ff292a60
5 changed files with 45 additions and 113 deletions
+1 -105
View File
@@ -408,7 +408,6 @@ steps:
from_secret: gcp_key
depends_on:
- build-storybook
- end-to-end-tests
- name: publish-frontend-metrics
image: grafana/build-container:1.4.3
@@ -500,7 +499,6 @@ steps:
GITHUB_PACKAGE_TOKEN:
from_secret: github_package_token
depends_on:
- end-to-end-tests
- end-to-end-tests-server
- name: upload-packages
@@ -512,7 +510,6 @@ steps:
from_secret: gcp_key
depends_on:
- package
- end-to-end-tests
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -828,16 +825,6 @@ steps:
depends_on:
- package
- name: end-to-end-tests
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3001 --tries 3
environment:
HOST: end-to-end-tests-server
depends_on:
- end-to-end-tests-server
- name: build-storybook
image: grafana/build-container:1.4.3
commands:
@@ -931,7 +918,6 @@ steps:
from_secret: gcp_key
depends_on:
- package
- end-to-end-tests
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -948,7 +934,6 @@ steps:
from_secret: gcp_key
depends_on:
- build-storybook
- end-to-end-tests
- name: release-npm-packages
image: grafana/build-container:1.4.3
@@ -1227,16 +1212,6 @@ steps:
depends_on:
- package
- name: end-to-end-tests
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3001 --tries 3
environment:
HOST: end-to-end-tests-server
depends_on:
- end-to-end-tests-server
- name: copy-packages-for-docker
image: grafana/build-container:1.4.3
commands:
@@ -1342,7 +1317,6 @@ steps:
from_secret: gcp_key
depends_on:
- package
- end-to-end-tests
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -1389,16 +1363,6 @@ steps:
depends_on:
- package-enterprise2
- name: end-to-end-tests-enterprise2
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3002 --tries 3
environment:
HOST: end-to-end-tests-server-enterprise2
depends_on:
- end-to-end-tests-server-enterprise2
- name: upload-packages-enterprise2
image: grafana/grafana-ci-deploy:1.3.1
commands:
@@ -1408,7 +1372,6 @@ steps:
from_secret: gcp_key
depends_on:
- package-enterprise2
- end-to-end-tests-enterprise2
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -1765,16 +1728,6 @@ steps:
depends_on:
- package
- name: end-to-end-tests
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3001 --tries 3
environment:
HOST: end-to-end-tests-server
depends_on:
- end-to-end-tests-server
- name: build-storybook
image: grafana/build-container:1.4.3
commands:
@@ -1862,7 +1815,6 @@ steps:
from_secret: gcp_key
depends_on:
- package
- end-to-end-tests
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -1876,7 +1828,6 @@ steps:
from_secret: gcp_key
depends_on:
- build-storybook
- end-to-end-tests
- name: release-npm-packages
image: grafana/build-container:1.4.3
@@ -2153,16 +2104,6 @@ steps:
depends_on:
- package
- name: end-to-end-tests
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3001 --tries 3
environment:
HOST: end-to-end-tests-server
depends_on:
- end-to-end-tests-server
- name: copy-packages-for-docker
image: grafana/build-container:1.4.3
commands:
@@ -2262,7 +2203,6 @@ steps:
from_secret: gcp_key
depends_on:
- package
- end-to-end-tests
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -2309,16 +2249,6 @@ steps:
depends_on:
- package-enterprise2
- name: end-to-end-tests-enterprise2
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3002 --tries 3
environment:
HOST: end-to-end-tests-server-enterprise2
depends_on:
- end-to-end-tests-server-enterprise2
- name: upload-packages-enterprise2
image: grafana/grafana-ci-deploy:1.3.1
commands:
@@ -2328,7 +2258,6 @@ steps:
from_secret: gcp_key
depends_on:
- package-enterprise2
- end-to-end-tests-enterprise2
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -2681,16 +2610,6 @@ steps:
depends_on:
- package
- name: end-to-end-tests
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3001 --tries 3
environment:
HOST: end-to-end-tests-server
depends_on:
- end-to-end-tests-server
- name: build-storybook
image: grafana/build-container:1.4.3
commands:
@@ -2778,7 +2697,6 @@ steps:
from_secret: gcp_key
depends_on:
- package
- end-to-end-tests
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -3037,16 +2955,6 @@ steps:
depends_on:
- package
- name: end-to-end-tests
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3001 --tries 3
environment:
HOST: end-to-end-tests-server
depends_on:
- end-to-end-tests-server
- name: build-storybook
image: grafana/build-container:1.4.3
commands:
@@ -3156,7 +3064,6 @@ steps:
from_secret: gcp_key
depends_on:
- package
- end-to-end-tests
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -3203,16 +3110,6 @@ steps:
depends_on:
- package-enterprise2
- name: end-to-end-tests-enterprise2
image: grafana/ci-e2e:12.19.0-1
commands:
- ./node_modules/.bin/cypress install
- ./bin/grabpl e2e-tests --port 3002 --tries 3
environment:
HOST: end-to-end-tests-server-enterprise2
depends_on:
- end-to-end-tests-server-enterprise2
- name: upload-packages-enterprise2
image: grafana/grafana-ci-deploy:1.3.1
commands:
@@ -3222,7 +3119,6 @@ steps:
from_secret: gcp_key
depends_on:
- package-enterprise2
- end-to-end-tests-enterprise2
- end-to-end-tests-server
- mysql-integration-tests
- postgres-integration-tests
@@ -3428,6 +3324,6 @@ get:
---
kind: signature
hmac: 2ba86c708134cd67f098f896776b1420bec631fa043a14a9991ee2167f0aff1e
hmac: b29b2b5d81bf443acb1f61caf900df9cba90b7e1a16fcc88aceeafcf7ad99895
...
+16 -3
View File
@@ -263,14 +263,27 @@ func (hs *HTTPServer) getPluginAssets(c *models.ReqContext) {
return
}
requestedFile := filepath.Clean(c.Params("*"))
pluginFilePath := filepath.Join(plugin.PluginDir, requestedFile)
// prepend slash for cleaning relative paths
requestedFile := filepath.Clean(filepath.Join("/", c.Params("*")))
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
+28
View File
@@ -24,9 +24,13 @@ func Test_GetPluginAssets(t *testing.T) {
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)
@@ -60,6 +64,30 @@ func Test_GetPluginAssets(t *testing.T) {
})
})
t.Run("Given a request for a relative path", func(t *testing.T) {
p := &plugins.PluginBase{
Id: pluginID,
PluginDir: pluginDir,
SignedFiles: map[string]struct{}{
requestedFile: {},
},
}
service := &pluginManager{
plugins: map[string]*plugins.PluginBase{
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.PluginBase{
Id: pluginID,
-3
View File
@@ -295,7 +295,6 @@ def publish_storybook_step(edition, ver_mode):
'image': publish_image,
'depends_on': [
'build-storybook',
'end-to-end-tests',
],
'environment': {
'GCP_KEY': from_secret('gcp_key'),
@@ -798,7 +797,6 @@ def release_canary_npm_packages_step(edition):
'name': 'release-canary-npm-packages',
'image': build_image,
'depends_on': [
'end-to-end-tests',
'end-to-end-tests-server',
],
'environment': {
@@ -848,7 +846,6 @@ def upload_packages_step(edition, ver_mode, is_downstream=False):
dependencies = [
'package' + enterprise2_sfx(edition),
'end-to-end-tests' + enterprise2_sfx(edition),
'end-to-end-tests-server',
'mysql-integration-tests',
'postgres-integration-tests',
-2
View File
@@ -97,7 +97,6 @@ def get_steps(edition, ver_mode):
gen_version_step(ver_mode=ver_mode, include_enterprise2=include_enterprise2),
package_step(edition=edition, ver_mode=ver_mode),
e2e_tests_server_step(edition=edition),
e2e_tests_step(edition=edition, tries=3),
build_storybook_step(edition=edition, ver_mode=ver_mode),
copy_packages_for_docker_step(),
build_docker_images_step(edition=edition, ver_mode=ver_mode, publish=should_publish),
@@ -125,7 +124,6 @@ def get_steps(edition, ver_mode):
package_step(edition=edition2, ver_mode=ver_mode, variants=['linux-x64']),
upload_cdn(edition=edition2),
e2e_tests_server_step(edition=edition2, port=3002),
e2e_tests_step(edition=edition2, port=3002, tries=3),
])
if should_upload:
steps.append(upload_packages_step(edition=edition2, ver_mode=ver_mode))