improved path traversal protection

Currently relative paths are validated against path traversals although
in an incomplete manner. While relative paths with traversals do not cause
problems for Gitaly in itself, we need be sure that every path accessed lies
within the storage directories to ensure RPC callers can't access arbitrary
paths. This commit replaces the path traversal checks by checking that the
relative paths refer to paths within the root of the storage or the storage
root itself.

1 files changed, 26 insertions, 13 deletions

diff --git a/internal/helper/security_test.go b/internal/helper/security_test.go
index 9a8125dac..6122f35a6 100644
--- a/internal/helper/security_test.go
+++ b/internal/helper/security_test.go
@@ -6,20 +6,33 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestContainsPathTraversal(t *testing.T) {
-	testCases := []struct {
-		path              string
-		containsTraversal bool
+func TestValidateRelativePath(t *testing.T) {
+	for _, tc := range []struct {
+		path    string
+		cleaned string
+		error   error
 	}{
-		{"../parent", true},
-		{"subdir/../../parent", true},
-		{"subdir/..", true},
-		{"subdir", false},
-		{"./subdir", false},
-	}
-
-	for _, tc := range testCases {
-		assert.Equal(t, tc.containsTraversal, ContainsPathTraversal(tc.path))
+		{"/parent", "parent", nil},
+		{"parent/", "parent", nil},
+		{"/parent-with-suffix", "parent-with-suffix", nil},
+		{"/subfolder", "subfolder", nil},
+		{"subfolder", "subfolder", nil},
+		{"subfolder/", "subfolder", nil},
+		{"subfolder//", "subfolder", nil},
+		{"subfolder/..", ".", nil},
+		{"subfolder/../..", "", ErrRelativePathEscapesRoot},
+		{"/..", "", ErrRelativePathEscapesRoot},
+		{"..", "", ErrRelativePathEscapesRoot},
+		{"../", "", ErrRelativePathEscapesRoot},
+		{"", ".", nil},
+		{".", ".", nil},
+	} {
+		const parent = "/parent"
+		t.Run(parent+" and "+tc.path, func(t *testing.T) {
+			cleaned, err := ValidateRelativePath(parent, tc.path)
+			assert.Equal(t, tc.cleaned, cleaned)
+			assert.Equal(t, tc.error, err)
+		})
 	}
 }