Enhance validation for body file path

core/cmd/hoverfly/main.go

@@ -27,6 +27,8 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"path"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
@@ -207,7 +209,7 @@ func main() {
 	flag.Var(&templatingDataSourceFlags, "templating-data-source", "Set template data source (i.e. '-templating-data-source \"<datasource name> <file path>\"')")
 	flag.Var(&destinationFlags, "dest", "Specify which hosts to process (i.e. '-dest fooservice.org -dest barservice.org -dest catservice.org') - other hosts will be ignored will passthrough'")
 	flag.Var(&logOutputFlags, "logs-output", "Specify locations for output logs, options are \"console\" and \"file\" (default \"console\")")
-	flag.StringVar(&responseBodyFilesPath, "response-body-files-path", "", "When a response contains a relative bodyFile, it will be resolved against this path (default is CWD)")
+	flag.StringVar(&responseBodyFilesPath, "response-body-files-path", "", "When a response contains a relative bodyFile, it will be resolved against this absolute path (default is CWD)")
 	flag.Var(&responseBodyFilesAllowedOriginFlags, "response-body-files-allow-origin", "When a response contains a url in bodyFile, it will be loaded only if the origin is allowed")
 	flag.Var(&journalIndexingKeyFlags, "journal-indexing-key", "Key to setup indexing on journal")
 
@@ -433,7 +435,24 @@ func main() {
 		cfg.Destination = *destination
 	}
 
-	cfg.ResponsesBodyFilesPath = responseBodyFilesPath
+
+	if len(responseBodyFilesPath) > 0 {
+		// Ensure file path is absolute and exists in the file system
+		if !path.IsAbs(responseBodyFilesPath) {
+			log.Fatal("Response body files path should be absolute")
+		}
+		absBasePath, err := filepath.Abs(responseBodyFilesPath)
+		if err != nil {
+			log.Fatal("Invalid response body files path")
+		}
+		if _, err := os.Stat(absBasePath); os.IsNotExist(err) {
+			log.Fatal("Response body files path does not exist")
+		}
+
+		cfg.ResponsesBodyFilesPath = absBasePath
+	}
+
+
 
 	for _, allowedOrigin := range responseBodyFilesAllowedOriginFlags {
 		if !util.IsURL(allowedOrigin) {

core/hoverfly_funcs.go

@@ -2,8 +2,8 @@ package hoverfly
 
 import (
 	"fmt"
-	"io/ioutil"
 	"net/http"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -188,7 +188,12 @@ func (hf *Hoverfly) readResponseBodyFile(filePath string) (string, error) {
 		return "", fmt.Errorf("bodyFile contains absolute path (%s). only relative is supported", filePath)
 	}
 
-	fileContents, err := ioutil.ReadFile(filepath.Join(hf.Cfg.ResponsesBodyFilesPath, filePath))
+	resolvedPath, err := util.ResolveAndValidatePath(hf.Cfg.ResponsesBodyFilesPath, filePath)
+	if err != nil {
+		return "", err
+	}
+
+	fileContents, err := os.ReadFile(resolvedPath)
 	if err != nil {
 		return "", err
 	}

core/util/util.go

@@ -17,6 +17,7 @@ import (
 	"math/rand"
 	"net/http"
 	"net/url"
+	"path/filepath"
 	"reflect"
 	"regexp"
 	"sort"
@@ -517,11 +518,27 @@ func NeedsEncoding(headers map[string][]string, body string) bool {
 	return needsEncoding
 }
 
-func IsJsonData(data string) bool {
-	var jsonData map[string]interface{}
-	if err := json.Unmarshal([]byte(data), &jsonData); err == nil {
-		return true
-	} else {
-		return false
+// Resolves a relative path from an absolute basePath, and fails if the relative path starts with ".."
+func ResolveAndValidatePath(absBasePath, relativePath string) (string, error) {
+
+	cleanRelativePath := filepath.Clean(relativePath)
+
+	// Check if the relative path starts with ".."
+	if strings.HasPrefix(cleanRelativePath, "..") {
+		return "", fmt.Errorf("relative path is invalid as it attempts to backtrack")
 	}
+
+	resolvedPath := filepath.Join(absBasePath, cleanRelativePath)
+
+	// Verify that the resolved path is indeed a subpath of the base path
+	finalPath, err := filepath.Rel(absBasePath, resolvedPath)
+	if err != nil {
+		return "", fmt.Errorf("failed to get relative path: %v", err)
+	}
+
+	if strings.HasPrefix(finalPath, "..") {
+		return "", fmt.Errorf("resolved path is outside the base path")
+	}
+
+	return resolvedPath, nil
 }

core/util/util_test.go

@@ -2,8 +2,10 @@ package util
 
 import (
 	"bytes"
-	"io/ioutil"
+	"io"
 	"net/http"
+	"os"
+	"path/filepath"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -13,7 +15,7 @@ func Test_GetRequestBody_GettingTheRequestBodyGetsTheCorrectData(t *testing.T) {
 	RegisterTestingT(t)
 
 	request := &http.Request{}
-	request.Body = ioutil.NopCloser(bytes.NewBuffer([]byte("test")))
+	request.Body = io.NopCloser(bytes.NewBuffer([]byte("test")))
 
 	requestBody, err := GetRequestBody(request)
 	Expect(err).To(BeNil())
@@ -25,12 +27,12 @@ func Test_GetRequestBody_GettingTheRequestBodySetsTheSameBodyAgain(t *testing.T)
 	RegisterTestingT(t)
 
 	request := &http.Request{}
-	request.Body = ioutil.NopCloser(bytes.NewBuffer([]byte("test-preserve")))
+	request.Body = io.NopCloser(bytes.NewBuffer([]byte("test-preserve")))
 
 	_, err := GetRequestBody(request)
 	Expect(err).To(BeNil())
 
-	newRequestBody, err := ioutil.ReadAll(request.Body)
+	newRequestBody, err := io.ReadAll(request.Body)
 	Expect(err).To(BeNil())
 
 	Expect(string(newRequestBody)).To(Equal("test-preserve"))
@@ -46,12 +48,12 @@ func Test_GetRequestBody_DecompressGzipContent(t *testing.T) {
 	compressedBody, err := CompressGzip([]byte(originalBody))
 	Expect(err).To(BeNil())
 	Expect(string(compressedBody)).To(Not(Equal(originalBody)))
-	request.Body = ioutil.NopCloser(bytes.NewBuffer(compressedBody))
+	request.Body = io.NopCloser(bytes.NewBuffer(compressedBody))
 
 	_, err = GetRequestBody(request)
 	Expect(err).To(BeNil())
 
-	newRequestBody, err := ioutil.ReadAll(request.Body)
+	newRequestBody, err := io.ReadAll(request.Body)
 	Expect(err).To(BeNil())
 
 	Expect(string(newRequestBody)).To(Equal(originalBody))
@@ -61,7 +63,7 @@ func Test_GetResponseBody_GettingTheResponseBodyGetsTheCorrectData(t *testing.T)
 	RegisterTestingT(t)
 
 	response := &http.Response{}
-	response.Body = ioutil.NopCloser(bytes.NewBuffer([]byte("test")))
+	response.Body = io.NopCloser(bytes.NewBuffer([]byte("test")))
 
 	responseBody, err := GetResponseBody(response)
 	Expect(err).To(BeNil())
@@ -74,12 +76,12 @@ func Test_GetResponseBody_GettingTheResponseBodySetsTheSameBodyAgain(t *testing.
 	RegisterTestingT(t)
 
 	response := &http.Response{}
-	response.Body = ioutil.NopCloser(bytes.NewBuffer([]byte("test-preserve")))
+	response.Body = io.NopCloser(bytes.NewBuffer([]byte("test-preserve")))
 
 	_, err := GetResponseBody(response)
 	Expect(err).To(BeNil())
 
-	newResponseBody, err := ioutil.ReadAll(response.Body)
+	newResponseBody, err := io.ReadAll(response.Body)
 	Expect(err).To(BeNil())
 
 	Expect(string(newResponseBody)).To(Equal("test-preserve"))
@@ -352,3 +354,44 @@ func Test_ContainsOnly_ReturnFalseWithOneExtraValue(t *testing.T) {
 	Expect(ContainsOnly(first[:], second[:])).To(BeFalse())
 
 }
+
+func TestResolveAndValidatePath(t *testing.T) {
+	RegisterTestingT(t)
+
+	cwd, err := os.Getwd()
+	Expect(err).NotTo(HaveOccurred())
+
+	tests := []struct {
+		basePath     string
+		relativePath string
+		expected     string
+		shouldErr    bool
+	}{
+		{"/home/user/project", "subdir/file.txt", "/home/user/project/subdir/file.txt", false},
+		{"/home/user/project", "../subdir/file.txt", "", true},
+		{"/home/user/project", "../../etc/passwd", "", true},
+		{"/home/user/project", "./subdir/file.txt", "/home/user/project/subdir/file.txt", false},
+		{"/home/user/project", "subdir/../file.txt", "/home/user/project/file.txt", false},
+		{"/home/user/project", ".", "/home/user/project", false},
+		{"", "subdir/file.txt", filepath.Join(cwd, "subdir/file.txt"), false},
+		{"", "", cwd, false},
+		{"", ".", cwd, false},
+		{"home/user/project", "subdir/file.txt", filepath.Join(cwd, "home/user/project/subdir/file.txt"), false},
+		{"./home/user/project", "subdir/file.txt", filepath.Join(cwd, "home/user/project/subdir/file.txt"), false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.relativePath, func(t *testing.T) {
+			result, err := ResolveAndValidatePath(test.basePath, test.relativePath)
+			if test.shouldErr {
+				Expect(err).To(HaveOccurred())
+			} else {
+				Expect(err).NotTo(HaveOccurred())
+				// Convert paths to absolute to avoid issues with relative paths in tests.
+				expectedAbs, _ := filepath.Abs(test.expected)
+				resultAbs, _ := filepath.Abs(result)
+				Expect(resultAbs).To(Equal(expectedAbs))
+			}
+		})
+	}
+}