Plugins: Fix files with two dots in the name not being returned by LocalFS.Files() (#67395)

* Fix files with two dots in the name not being returned by LocalFS.Files()

* Renamed variable for consistency

* Add test

* Fix typo

* Fix wrong upperLevelPrefix value

* Removed unnecessary check in LocalFS.Files()
This commit is contained in:
Giuseppe Guerra 2023-04-27 16:19:13 +02:00 committed by GitHub
parent 4b241311b3
commit 7c5210a915
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 27 additions and 3 deletions

View File

@ -34,6 +34,7 @@ func NewLocalFS(basePath string) LocalFS {
// file is allowed or not. Access to a file is allowed if the file is in the FS's Base() directory, and if it's a
// symbolic link it should not end up outside the plugin's directory.
func (f LocalFS) fileIsAllowed(basePath string, absolutePath string, info os.FileInfo) (bool, error) {
upperLevelPrefix := ".." + string(filepath.Separator)
if info.Mode()&os.ModeSymlink == os.ModeSymlink {
symlinkPath, err := filepath.EvalSymlinks(absolutePath)
if err != nil {
@ -50,7 +51,7 @@ func (f LocalFS) fileIsAllowed(basePath string, absolutePath string, info os.Fil
if err != nil {
return false, err
}
if p == ".." || strings.HasPrefix(p, ".."+string(filepath.Separator)) {
if p == ".." || strings.HasPrefix(p, upperLevelPrefix) {
return false, fmt.Errorf("file '%s' not inside of plugin directory", p)
}
@ -70,7 +71,7 @@ func (f LocalFS) fileIsAllowed(basePath string, absolutePath string, info os.Fil
if err != nil {
return false, err
}
if strings.HasPrefix(file, ".."+string(filepath.Separator)) {
if strings.HasPrefix(file, upperLevelPrefix) {
return false, fmt.Errorf("file '%s' not inside of plugin directory", file)
}
return true, nil
@ -147,7 +148,7 @@ func (f LocalFS) Files() ([]string, error) {
return nil, err
}
clenRelPath, err := util.CleanRelativePath(relPath)
if strings.Contains(clenRelPath, "..") || err != nil {
if err != nil {
continue
}
relFiles = append(relFiles, clenRelPath)

View File

@ -277,3 +277,26 @@ func TestStaticFS(t *testing.T) {
})
})
}
// TestFSTwoDotsInFileName ensures that LocalFS and StaticFS allow two dots in file names.
// This makes sure that FSes do not believe that two dots in a file name (anywhere in the path)
// represent a path traversal attempt.
func TestFSTwoDotsInFileName(t *testing.T) {
tmp := t.TempDir()
const fn = "test..png"
require.NoError(t, createDummyTempFile(tmp, fn))
localFS := NewLocalFS(tmp)
staticFS, err := NewStaticFS(localFS)
require.NoError(t, err)
// Test both with localFS and staticFS
files, err := localFS.Files()
require.NoError(t, err)
require.Equal(t, []string{"test..png"}, files)
files, err = staticFS.Files()
require.NoError(t, err)
require.Equal(t, []string{"test..png"}, files)
}