Validate file references

4 files changed, 47 insertions, 6 deletions

diff --git a/client/go/cmd/test.go b/client/go/cmd/test.go
index c2aaab68a0c..f323a0a76cc 100644
--- a/client/go/cmd/test.go
+++ b/client/go/cmd/test.go
@@ -17,7 +17,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 	"time"
@@ -77,7 +76,7 @@ func runTests(rootPath string, target vespa.Target, dryRun bool) (int, []string)
 		previousFailed := false
 		for _, test := range tests {
 			if !test.IsDir() && filepath.Ext(test.Name()) == ".json" {
-				testPath := path.Join(rootPath, test.Name())
+				testPath := filepath.Join(rootPath, test.Name())
 				if previousFailed {
 					fmt.Fprintln(stdout, "")
 					previousFailed = false
@@ -122,7 +121,7 @@ func runTest(testPath string, target vespa.Target, dryRun bool) string {
 		fmt.Fprintf(stdout, "%s:", testName)
 	}
 
-	defaultParameters, err := getParameters(test.Defaults.ParametersRaw, path.Dir(testPath))
+	defaultParameters, err := getParameters(test.Defaults.ParametersRaw, filepath.Dir(testPath))
 	if err != nil {
 		fmt.Fprintln(stderr)
 		fatalErrHint(err, fmt.Sprintf("Invalid default parameters for %s", testName), "See https://cloud.vespa.ai/en/reference/testing")
@@ -137,7 +136,7 @@ func runTest(testPath string, target vespa.Target, dryRun bool) string {
 		if step.Name != "" {
 			stepName = fmt.Sprintf("Step: %s", step.Name)
 		}
-		failure, longFailure, err := verify(step, path.Dir(testPath), test.Defaults.Cluster, defaultParameters, target, dryRun)
+		failure, longFailure, err := verify(step, filepath.Dir(testPath), test.Defaults.Cluster, defaultParameters, target, dryRun)
 		if err != nil {
 			fmt.Fprintln(stderr)
 			fatalErrHint(err, fmt.Sprintf("Error in %s", stepName), "See https://cloud.vespa.ai/en/reference/testing")
@@ -383,7 +382,10 @@ func getParameters(parametersRaw []byte, testsPath string) (map[string]string, e
 	if parametersRaw != nil {
 		var parametersPath string
 		if err := json.Unmarshal(parametersRaw, &parametersPath); err == nil {
-			resolvedParametersPath := path.Join(testsPath, parametersPath)
+			if err = validateRelativePath(parametersPath); err != nil {
+				return nil, err
+			}
+			resolvedParametersPath := filepath.Join(testsPath, parametersPath)
 			parametersRaw, err = ioutil.ReadFile(resolvedParametersPath)
 			if err != nil {
 				return nil, fmt.Errorf("failed to read request parameters at %s: %w", resolvedParametersPath, err)
@@ -401,7 +403,10 @@ func getParameters(parametersRaw []byte, testsPath string) (map[string]string, e
 func getBody(bodyRaw []byte, testsPath string) ([]byte, error) {
 	var bodyPath string
 	if err := json.Unmarshal(bodyRaw, &bodyPath); err == nil {
-		resolvedBodyPath := path.Join(testsPath, bodyPath)
+		if err = validateRelativePath(bodyPath); err != nil {
+			return nil, err
+		}
+		resolvedBodyPath := filepath.Join(testsPath, bodyPath)
 		bodyRaw, err = ioutil.ReadFile(resolvedBodyPath)
 		if err != nil {
 			return nil, fmt.Errorf("failed to read body file at %s: %w", resolvedBodyPath, err)
@@ -410,6 +415,18 @@ func getBody(bodyRaw []byte, testsPath string) ([]byte, error) {
 	return bodyRaw, nil
 }
 
+func validateRelativePath(relPath string) error {
+	if filepath.IsAbs(relPath) {
+		return fmt.Errorf("path must be relative, but was '%s'", relPath)
+	}
+	cleanPath := filepath.Clean(relPath)
+	fmt.Println(cleanPath)
+	if strings.HasPrefix(cleanPath, "../../../") {
+		return fmt.Errorf("path may not point outside src/test/application, but '%s' does", relPath)
+	}
+	return nil
+}
+
 type test struct {
 	Name     string   `json:"name"`
 	Defaults defaults `json:"defaults"`
diff --git a/client/go/cmd/test_test.go b/client/go/cmd/test_test.go
index 10a62e0495d..6649353df77 100644
--- a/client/go/cmd/test_test.go
+++ b/client/go/cmd/test_test.go
@@ -45,6 +45,15 @@ func TestSuite(t *testing.T) {
 	assert.Equal(t, "", errBytes)
 }
 
+func TestIllegalFileReference(t *testing.T) {
+	client := &mockHttpClient{}
+	client.NextStatus(200)
+	client.NextStatus(200)
+	_, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/illegal-reference.json"}}, t, client)
+	assertRequests([]*http.Request{createRequest("GET", "http://127.0.0.1:8080/search/", "{}")}, client, t)
+	assert.Equal(t, "\nError: path may not point outside src/test/application, but 'foo/../../../../this-is-not-ok.json' does\nHint: Error in Step 2\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes)
+}
+
 func TestProductionTest(t *testing.T) {
 	client := &mockHttpClient{}
 	client.NextStatus(200)
diff --git a/client/go/cmd/testdata/empty.json b/client/go/cmd/testdata/empty.json
new file mode 100644
index 00000000000..9e26dfeeb6e
--- /dev/null
+++ b/client/go/cmd/testdata/empty.json
@@ -0,0 +1 @@
+{}
\ No newline at end of file
diff --git a/client/go/cmd/testdata/tests/production-test/illegal-reference.json b/client/go/cmd/testdata/tests/production-test/illegal-reference.json
new file mode 100644
index 00000000000..edd8a2fafeb
--- /dev/null
+++ b/client/go/cmd/testdata/tests/production-test/illegal-reference.json
@@ -0,0 +1,14 @@
+{
+    "steps": [
+        {
+            "request": {
+                "body": "foo/../../../empty.json"
+            }
+        },
+        {
+            "request": {
+                "body": "foo/../../../../this-is-not-ok.json"
+            }
+        }
+    ]
+}
\ No newline at end of file