diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2020-04-30 18:36:18 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2020-04-30 19:07:50 +0300 |
commit | f89b33baaa4b34db9444d92466921c1e4a0a66f5 (patch) | |
tree | d88c70165f0caec583463014f1791f8ea75824c0 /internal/helper/security_test.go | |
parent | 9bfdd53b6b9beca5f88500c8dd12d031d0fb6bc9 (diff) |
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.
Diffstat (limited to 'internal/helper/security_test.go')
-rw-r--r-- | internal/helper/security_test.go | 39 |
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) + }) } } |