From 7c5210a91591bee86b678bf898e65a52618ebc5a Mon Sep 17 00:00:00 2001
From: Giuseppe Guerra <giuseppe.guerra@grafana.com>
Date: Thu, 27 Apr 2023 16:19:13 +0200
Subject: [PATCH] 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()
---
 pkg/plugins/localfiles.go      |  7 ++++---
 pkg/plugins/localfiles_test.go | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/pkg/plugins/localfiles.go b/pkg/plugins/localfiles.go
index 8def6dd7503..5981d13256e 100644
--- a/pkg/plugins/localfiles.go
+++ b/pkg/plugins/localfiles.go
@@ -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)
diff --git a/pkg/plugins/localfiles_test.go b/pkg/plugins/localfiles_test.go
index af523cde9b7..8ad5903cd69 100644
--- a/pkg/plugins/localfiles_test.go
+++ b/pkg/plugins/localfiles_test.go
@@ -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)
+}