Prechádzať zdrojové kódy

fix: prevent path traversal via edge-level path normalization

Moved path sanitization (backslash conversion and path cleaning) to
the SFTP/FTP handlers before VFS routing and permission checks.

Signed-off-by: Nicola Murino <[email protected]>
Nicola Murino 1 mesiac pred
rodič
commit
2f092d1289

+ 345 - 0
internal/common/connection_test.go

@@ -1189,3 +1189,348 @@ func TestListerAt(t *testing.T) {
 	err = lister.Close()
 	require.NoError(t, err)
 }
+
+func TestGetFsAndResolvedPath(t *testing.T) {
+	homeDir := filepath.Join(os.TempDir(), "home_test")
+	localVdir := filepath.Join(os.TempDir(), "local_mount_test")
+
+	err := os.MkdirAll(homeDir, 0777)
+	require.NoError(t, err)
+	err = os.MkdirAll(localVdir, 0777)
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		os.RemoveAll(homeDir)
+		os.RemoveAll(localVdir)
+	})
+
+	user := dataprovider.User{
+		BaseUser: sdk.BaseUser{
+			Username: xid.New().String(),
+			Status:   1,
+			HomeDir:  homeDir,
+		},
+		VirtualFolders: []vfs.VirtualFolder{
+			{
+				BaseVirtualFolder: vfs.BaseVirtualFolder{
+					Name:       "s3",
+					MappedPath: "",
+					FsConfig: vfs.Filesystem{
+						Provider: sdk.S3FilesystemProvider,
+						S3Config: vfs.S3FsConfig{
+							BaseS3FsConfig: sdk.BaseS3FsConfig{
+								Bucket: "my-test-bucket",
+								Region: "us-east-1",
+							},
+						},
+					},
+				},
+				VirtualPath: "/s3",
+			},
+			{
+				BaseVirtualFolder: vfs.BaseVirtualFolder{
+					Name:       "local",
+					MappedPath: localVdir,
+					FsConfig: vfs.Filesystem{
+						Provider: sdk.LocalFilesystemProvider,
+					},
+				},
+				VirtualPath: "/local",
+			},
+		},
+	}
+
+	conn := NewBaseConnection(xid.New().String(), ProtocolSFTP, "", "", user)
+
+	tests := []struct {
+		name                 string
+		inputVirtualPath     string
+		expectedFsType       string
+		expectedPhyPath      string // The resolved path on the target FS
+		expectedRelativePath string
+	}{
+		{
+			name:                 "Root File",
+			inputVirtualPath:     "/file.txt",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(homeDir, "file.txt"),
+			expectedRelativePath: "/file.txt",
+		},
+		{
+			name:                 "Standard S3 File",
+			inputVirtualPath:     "/s3/image.png",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "image.png",
+			expectedRelativePath: "/s3/image.png",
+		},
+		{
+			name:                 "Standard Local Mount File",
+			inputVirtualPath:     "/local/config.json",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(localVdir, "config.json"),
+			expectedRelativePath: "/local/config.json",
+		},
+
+		{
+			name:                 "Backslash Separator -> Should hit S3",
+			inputVirtualPath:     "\\s3\\doc.txt",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "doc.txt",
+			expectedRelativePath: "/s3/doc.txt",
+		},
+		{
+			name:                 "Mixed Separators -> Should hit Local Mount",
+			inputVirtualPath:     "/local\\subdir/test.txt",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(localVdir, "subdir", "test.txt"),
+			expectedRelativePath: "/local/subdir/test.txt",
+		},
+		{
+			name:                 "Double Slash -> Should normalize and hit S3",
+			inputVirtualPath:     "//s3//dir @1/data.csv",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "dir @1/data.csv",
+			expectedRelativePath: "/s3/dir @1/data.csv",
+		},
+
+		{
+			name:                 "Local Mount Traversal (Attempt to escape)",
+			inputVirtualPath:     "/local/../../etc/passwd",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(homeDir, "/etc/passwd"),
+			expectedRelativePath: "/etc/passwd",
+		},
+		{
+			name:                 "Traversal Out of S3 (Valid)",
+			inputVirtualPath:     "/s3/../../secret.txt",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(homeDir, "secret.txt"),
+			expectedRelativePath: "/secret.txt",
+		},
+		{
+			name:                 "Traversal Inside S3",
+			inputVirtualPath:     "/s3/subdir/../image.png",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "image.png",
+			expectedRelativePath: "/s3/image.png",
+		},
+		{
+			name:                 "Mount Point Bypass -> Target Local Mount",
+			inputVirtualPath:     "/s3\\..\\local\\secret.txt",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(localVdir, "secret.txt"),
+			expectedRelativePath: "/local/secret.txt",
+		},
+		{
+			name:                 "Dirty Relative Path (Your Case)",
+			inputVirtualPath:     "test\\..\\..\\oops/file.txt",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(homeDir, "oops", "file.txt"),
+			expectedRelativePath: "/oops/file.txt",
+		},
+		{
+			name:                 "Relative Path targeting S3 (No leading slash)",
+			inputVirtualPath:     "s3//sub/../image.png",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "image.png",
+			expectedRelativePath: "/s3/image.png",
+		},
+		{
+			name:                 "Windows Path starting with Backslash",
+			inputVirtualPath:     "\\s3\\doc/dir\\doc.txt",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "doc/dir/doc.txt",
+			expectedRelativePath: "/s3/doc/dir/doc.txt",
+		},
+		{
+			name:                 "Filesystem Juggling (Relative)",
+			inputVirtualPath:     "local/../s3/file.txt",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "file.txt",
+			expectedRelativePath: "/s3/file.txt",
+		},
+		{
+			name:                 "Triple Dot Filename (Valid Name)",
+			inputVirtualPath:     "/...hidden/secret",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(homeDir, "...hidden", "secret"),
+			expectedRelativePath: "/...hidden/secret",
+		},
+		{
+			name:                 "Dot Slash Prefix",
+			inputVirtualPath:     "./local/file.txt",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      filepath.Join(localVdir, "file.txt"),
+			expectedRelativePath: "/local/file.txt",
+		},
+		{
+			name:                 "Root of Local Mount Exactly",
+			inputVirtualPath:     "/local/",
+			expectedFsType:       "osfs",
+			expectedPhyPath:      localVdir,
+			expectedRelativePath: "/local",
+		},
+		{
+			name:                 "Root of S3 Mount Exactly",
+			inputVirtualPath:     "/s3/",
+			expectedFsType:       "S3Fs",
+			expectedPhyPath:      "",
+			expectedRelativePath: "/s3",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// The input path is sanitized by the protocol handler
+			// implementations before reaching GetFsAndResolvedPath.
+			cleanInput := util.CleanPath(tc.inputVirtualPath)
+			fs, resolvedPath, err := conn.GetFsAndResolvedPath(cleanInput)
+			if assert.NoError(t, err, "did not expect error for path: %q, got: %v", tc.inputVirtualPath, err) {
+				assert.Contains(t, fs.Name(), tc.expectedFsType,
+					"routing error: input %q but expected fs %q, got %q", tc.inputVirtualPath, tc.expectedFsType, fs.Name())
+				assert.Equal(t, tc.expectedPhyPath, resolvedPath,
+					"resolution error: input %q resolved to %q expected %q", tc.inputVirtualPath, resolvedPath, tc.expectedPhyPath)
+				relativePath := fs.GetRelativePath(resolvedPath)
+				assert.Equal(t, tc.expectedRelativePath, relativePath,
+					"relative path error, input %q, got %q, expected %q", tc.inputVirtualPath, tc.expectedRelativePath, relativePath)
+			}
+		})
+	}
+}
+
+func TestOsFsGetRelativePath(t *testing.T) {
+	homeDir := filepath.Join(os.TempDir(), "home_test")
+	localVdir := filepath.Join(os.TempDir(), "local_mount_test")
+
+	err := os.MkdirAll(homeDir, 0777)
+	require.NoError(t, err)
+	err = os.MkdirAll(localVdir, 0777)
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		os.RemoveAll(homeDir)
+		os.RemoveAll(localVdir)
+	})
+
+	user := dataprovider.User{
+		BaseUser: sdk.BaseUser{
+			Username: xid.New().String(),
+			Status:   1,
+			HomeDir:  homeDir,
+		},
+		VirtualFolders: []vfs.VirtualFolder{
+			{
+				BaseVirtualFolder: vfs.BaseVirtualFolder{
+					Name:       "local",
+					MappedPath: localVdir,
+					FsConfig: vfs.Filesystem{
+						Provider: sdk.LocalFilesystemProvider,
+					},
+				},
+				VirtualPath: "/local",
+			},
+		},
+	}
+
+	connID := xid.New().String()
+	rootFs, err := user.GetFilesystemForPath("/", connID)
+	require.NoError(t, err)
+
+	localFs, err := user.GetFilesystemForPath("/local", connID)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name        string
+		fs          vfs.Fs
+		inputPath   string // The physical path to reverse-map
+		expectedRel string // The expected virtual path
+	}{
+		{
+			name:        "Root FS - Inside root",
+			fs:          rootFs,
+			inputPath:   filepath.Join(homeDir, "docs", "file.txt"),
+			expectedRel: "/docs/file.txt",
+		},
+		{
+			name:        "Root FS - Exact root directory",
+			fs:          rootFs,
+			inputPath:   homeDir,
+			expectedRel: "/",
+		},
+		{
+			name:        "Root FS - External absolute path (Jail to /)",
+			fs:          rootFs,
+			inputPath:   "/etc/passwd",
+			expectedRel: "/",
+		},
+		{
+			name:        "Root FS - Traversal escape (Jail to /)",
+			fs:          rootFs,
+			inputPath:   filepath.Join(homeDir, "..", "escaped.txt"),
+			expectedRel: "/",
+		},
+		{
+			name:        "Root FS - Valid file named with triple dots",
+			fs:          rootFs,
+			inputPath:   filepath.Join(homeDir, "..."),
+			expectedRel: "/...",
+		},
+		{
+			name:        "Local FS - Up path in dir",
+			fs:          rootFs,
+			inputPath:   homeDir + "/../" + filepath.Base(homeDir) + "/dir/test.txt",
+			expectedRel: "/dir/test.txt",
+		},
+
+		{
+			name:        "Local FS - Inside mount",
+			fs:          localFs,
+			inputPath:   filepath.Join(localVdir, "data", "config.json"),
+			expectedRel: "/local/data/config.json",
+		},
+		{
+			name:        "Local FS - Exact mount directory",
+			fs:          localFs,
+			inputPath:   localVdir,
+			expectedRel: "/local",
+		},
+		{
+			name:        "Local FS - External absolute path (Jail to /local)",
+			fs:          localFs,
+			inputPath:   "/var/log/syslog",
+			expectedRel: "/local",
+		},
+		{
+			name:        "Local FS - Traversal escape (Jail to /local)",
+			fs:          localFs,
+			inputPath:   filepath.Join(localVdir, "..", "..", "etc", "passwd"),
+			expectedRel: "/local",
+		},
+		{
+			name:        "Local FS - Partial prefix (Jail to /local)",
+			fs:          localFs,
+			inputPath:   localVdir + "_backup",
+			expectedRel: "/local",
+		},
+		{
+			name:        "Local FS - Relative traversal matching virual dir",
+			fs:          localFs,
+			inputPath:   localVdir + "/../" + filepath.Base(localVdir) + "/dir/test.txt",
+			expectedRel: "/local/dir/test.txt",
+		},
+		{
+			name:        "Local FS - Valid file starting with two dots",
+			fs:          localFs,
+			inputPath:   filepath.Join(localVdir, "..hidden_file.txt"),
+			expectedRel: "/local/..hidden_file.txt",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			actualRel := tc.fs.GetRelativePath(tc.inputPath)
+			assert.Equal(t, tc.expectedRel, actualRel,
+				"Failed mapping physical path %q on FS %q", tc.inputPath, tc.fs.Name())
+		})
+	}
+}

+ 3 - 3
internal/common/protocol_test.go

@@ -7017,7 +7017,7 @@ func TestEventRuleRenameEvent(t *testing.T) {
 	assert.NoError(t, err)
 
 	u := getTestUser()
-	u.Username = "test <html > chars"
+	u.Username = "test & chars"
 	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
 	assert.NoError(t, err)
 	conn, client, err := getSftpClient(user)
@@ -7042,7 +7042,7 @@ func TestEventRuleRenameEvent(t *testing.T) {
 		assert.Contains(t, email.Data, fmt.Sprintf(`Subject: "rename" from "%s"`, user.Username))
 		assert.Contains(t, email.Data, "Content-Type: text/html")
 		assert.Contains(t, email.Data, fmt.Sprintf("Target path %q", path.Join("/subdir", testFileName)))
-		assert.Contains(t, email.Data, "Name: test &lt;html &gt; chars,")
+		assert.Contains(t, email.Data, "Name: test &amp; chars,")
 	}
 
 	_, err = httpdtest.RemoveEventRule(rule1, http.StatusOK)
@@ -7070,7 +7070,7 @@ func TestEventRuleIDPLogin(t *testing.T) {
 	require.NoError(t, err)
 	lastReceivedEmail.reset()
 
-	username := `test_"idp_"login`
+	username := `test_'idp_'login`
 	custom1 := `cust"oa"1`
 	u := map[string]any{
 		"username": "{{.Name}}",

+ 15 - 1
internal/ftpd/handler.go

@@ -29,6 +29,7 @@ import (
 	"github.com/drakkan/sftpgo/v2/internal/common"
 	"github.com/drakkan/sftpgo/v2/internal/dataprovider"
 	"github.com/drakkan/sftpgo/v2/internal/logger"
+	"github.com/drakkan/sftpgo/v2/internal/util"
 	"github.com/drakkan/sftpgo/v2/internal/vfs"
 )
 
@@ -97,6 +98,7 @@ func (c *Connection) Create(_ string) (afero.File, error) {
 // Mkdir creates a directory using the connection filesystem
 func (c *Connection) Mkdir(name string, _ os.FileMode) error {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 
 	return c.CreateDir(name, true)
 }
@@ -120,6 +122,7 @@ func (c *Connection) OpenFile(_ string, _ int, _ os.FileMode) (afero.File, error
 // We implements ClientDriverExtensionRemoveDir for directories
 func (c *Connection) Remove(name string) error {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 
 	fs, p, err := c.GetFsAndResolvedPath(name)
 	if err != nil {
@@ -147,6 +150,8 @@ func (c *Connection) RemoveAll(_ string) error {
 // Rename renames a file or a directory
 func (c *Connection) Rename(oldname, newname string) error {
 	c.UpdateLastActivity()
+	oldname = util.CleanPath(oldname)
+	newname = util.CleanPath(newname)
 
 	return c.BaseConnection.Rename(oldname, newname)
 }
@@ -155,6 +160,7 @@ func (c *Connection) Rename(oldname, newname string) error {
 // if any happens
 func (c *Connection) Stat(name string) (os.FileInfo, error) {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 	c.doWildcardListDir = false
 
 	if !c.User.HasPerm(dataprovider.PermListItems, path.Dir(name)) {
@@ -198,6 +204,7 @@ func (c *Connection) Chown(_ string, _, _ int) error {
 // Chmod changes the mode of the named file/directory
 func (c *Connection) Chmod(name string, mode os.FileMode) error {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 
 	attrs := common.StatAttributes{
 		Flags: common.StatAttrPerms,
@@ -209,6 +216,7 @@ func (c *Connection) Chmod(name string, mode os.FileMode) error {
 // Chtimes changes the access and modification times of the named file
 func (c *Connection) Chtimes(name string, atime time.Time, mtime time.Time) error {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 
 	attrs := common.StatAttributes{
 		Flags: common.StatAttrTimes,
@@ -221,6 +229,7 @@ func (c *Connection) Chtimes(name string, atime time.Time, mtime time.Time) erro
 // GetAvailableSpace implements ClientDriverExtensionAvailableSpace interface
 func (c *Connection) GetAvailableSpace(dirName string) (int64, error) {
 	c.UpdateLastActivity()
+	dirName = util.CleanPath(dirName)
 
 	diskQuota, transferQuota := c.HasSpace(false, false, path.Join(dirName, "fakefile.txt"))
 	if !diskQuota.HasSpace || !transferQuota.HasUploadSpace() {
@@ -279,6 +288,7 @@ func (c *Connection) AllocateSpace(_ int) error {
 // RemoveDir implements ClientDriverExtensionRemoveDir
 func (c *Connection) RemoveDir(name string) error {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 
 	return c.BaseConnection.RemoveDir(name)
 }
@@ -286,6 +296,8 @@ func (c *Connection) RemoveDir(name string) error {
 // Symlink implements ClientDriverExtensionSymlink
 func (c *Connection) Symlink(oldname, newname string) error {
 	c.UpdateLastActivity()
+	oldname = util.CleanPath(oldname)
+	newname = util.CleanPath(newname)
 
 	return c.CreateSymlink(oldname, newname)
 }
@@ -293,6 +305,7 @@ func (c *Connection) Symlink(oldname, newname string) error {
 // ReadDir implements ClientDriverExtensionFilelist
 func (c *Connection) ReadDir(name string) ([]os.FileInfo, error) {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 
 	if c.doWildcardListDir {
 		c.doWildcardListDir = false
@@ -311,7 +324,7 @@ func (c *Connection) ReadDir(name string) ([]os.FileInfo, error) {
 			pattern:        baseName,
 			lastCommand:    c.clientContext.GetLastCommand(),
 			dirName:        name,
-			connectionPath: c.clientContext.Path(),
+			connectionPath: util.CleanPath(c.clientContext.Path()),
 		}
 		return consumeDirLister(patternLister)
 	}
@@ -326,6 +339,7 @@ func (c *Connection) ReadDir(name string) ([]os.FileInfo, error) {
 // GetHandle implements ClientDriverExtentionFileTransfer
 func (c *Connection) GetHandle(name string, flags int, offset int64) (ftpserver.FileTransfer, error) {
 	c.UpdateLastActivity()
+	name = util.CleanPath(name)
 
 	fs, p, err := c.GetFsAndResolvedPath(name)
 	if err != nil {

+ 1 - 1
internal/httpd/oidc_test.go

@@ -1184,7 +1184,7 @@ func TestOIDCEvMgrIntegration(t *testing.T) {
 	err = dataprovider.Initialize(newProviderConf, configDir, true)
 	assert.NoError(t, err)
 	// add a special chars to check json replacer
-	username := `test_"oidc_eventmanager`
+	username := `test_'oidc_eventmanager`
 	u := map[string]any{
 		"username": "{{.Name}}",
 		"status":   1,

+ 23 - 4
internal/sftpd/handler.go

@@ -19,6 +19,7 @@ import (
 	"net"
 	"os"
 	"path"
+	"strings"
 	"time"
 
 	"github.com/pkg/sftp"
@@ -72,6 +73,7 @@ func (c *Connection) GetCommand() string {
 // Fileread creates a reader for a file on the system and returns the reader back.
 func (c *Connection) Fileread(request *sftp.Request) (io.ReaderAt, error) {
 	c.UpdateLastActivity()
+	updateRequestPaths(request)
 
 	if !c.User.HasPerm(dataprovider.PermDownload, path.Dir(request.Filepath)) {
 		return nil, sftp.ErrSSHFxPermissionDenied
@@ -126,6 +128,7 @@ func (c *Connection) Filewrite(request *sftp.Request) (io.WriterAt, error) {
 
 func (c *Connection) handleFilewrite(request *sftp.Request) (sftp.WriterAtReaderAt, error) { //nolint:gocyclo
 	c.UpdateLastActivity()
+	updateRequestPaths(request)
 
 	if err := common.Connections.IsNewTransferAllowed(c.User.Username); err != nil {
 		c.Log(logger.LevelInfo, "denying file write due to transfer count limits")
@@ -189,6 +192,7 @@ func (c *Connection) handleFilewrite(request *sftp.Request) (sftp.WriterAtReader
 // or writing to those files.
 func (c *Connection) Filecmd(request *sftp.Request) error {
 	c.UpdateLastActivity()
+	updateRequestPaths(request)
 
 	switch request.Method {
 	case "Setstat":
@@ -221,6 +225,7 @@ func (c *Connection) Filecmd(request *sftp.Request) error {
 // a directory as well as perform file/folder stat calls.
 func (c *Connection) Filelist(request *sftp.Request) (sftp.ListerAt, error) {
 	c.UpdateLastActivity()
+	updateRequestPaths(request)
 
 	switch request.Method {
 	case "List":
@@ -252,6 +257,7 @@ func (c *Connection) Filelist(request *sftp.Request) (sftp.ListerAt, error) {
 
 // Readlink implements the ReadlinkFileLister interface
 func (c *Connection) Readlink(filePath string) (string, error) {
+	filePath = util.CleanPath(filePath)
 	if err := c.canReadLink(filePath); err != nil {
 		return "", err
 	}
@@ -276,6 +282,7 @@ func (c *Connection) Readlink(filePath string) (string, error) {
 // Lstat implements LstatFileLister interface
 func (c *Connection) Lstat(request *sftp.Request) (sftp.ListerAt, error) {
 	c.UpdateLastActivity()
+	updateRequestPaths(request)
 
 	if !c.User.HasPerm(dataprovider.PermListItems, path.Dir(request.Filepath)) {
 		return nil, sftp.ErrSSHFxPermissionDenied
@@ -291,15 +298,14 @@ func (c *Connection) Lstat(request *sftp.Request) (sftp.ListerAt, error) {
 
 // RealPath implements the RealPathFileLister interface
 func (c *Connection) RealPath(p string) (string, error) {
-	if !c.User.HasPerm(dataprovider.PermListItems, path.Dir(p)) {
-		return "", sftp.ErrSSHFxPermissionDenied
-	}
-
 	if c.User.Filters.StartDirectory == "" {
 		p = util.CleanPath(p)
 	} else {
 		p = util.CleanPathWithBase(c.User.Filters.StartDirectory, p)
 	}
+	if !c.User.HasPerm(dataprovider.PermListItems, path.Dir(p)) {
+		return "", sftp.ErrSSHFxPermissionDenied
+	}
 	fs, fsPath, err := c.GetFsAndResolvedPath(p)
 	if err != nil {
 		return "", err
@@ -317,6 +323,7 @@ func (c *Connection) RealPath(p string) (string, error) {
 // StatVFS implements StatVFSFileCmder interface
 func (c *Connection) StatVFS(r *sftp.Request) (*sftp.StatVFS, error) {
 	c.UpdateLastActivity()
+	updateRequestPaths(r)
 
 	// we are assuming that r.Filepath is a dir, this could be wrong but should
 	// not produce any side effect here.
@@ -596,3 +603,15 @@ func getOSOpenFlags(requestFlags sftp.FileOpenFlags) (flags int) {
 	}
 	return osFlags
 }
+
+func updateRequestPaths(request *sftp.Request) {
+	if request.Method == "Symlink" {
+		request.Filepath = path.Clean(strings.ReplaceAll(request.Filepath, "\\", "/"))
+	} else {
+		request.Filepath = util.CleanPath(request.Filepath)
+	}
+
+	if request.Target != "" {
+		request.Target = util.CleanPath(request.Target)
+	}
+}

+ 3 - 2
internal/sftpd/internal_test.go

@@ -377,8 +377,9 @@ func TestWithInvalidHome(t *testing.T) {
 	c := Connection{
 		BaseConnection: common.NewBaseConnection("", common.ProtocolSFTP, "", "", u),
 	}
-	_, err = fs.ResolvePath("../upper_path")
-	assert.Error(t, err, "tested path is not a home subdir")
+	resolved, err := fs.ResolvePath("../upper_path")
+	assert.NoError(t, err)
+	assert.Equal(t, filepath.Join(u.HomeDir, "upper_path"), resolved)
 	_, err = c.StatVFS(&sftp.Request{
 		Method:   "StatVFS",
 		Filepath: "../unresolvable-path",

+ 4 - 10
internal/sftpd/sftpd_test.go

@@ -8584,18 +8584,12 @@ func TestResolvePaths(t *testing.T) {
 		assert.Equal(t, fs.Join(user.GetHomeDir(), "/test/sub"), resolved)
 		path = "../test/sub"
 		resolved, err = fs.ResolvePath(filepath.ToSlash(path))
-		if vfs.IsLocalOsFs(fs) {
-			assert.Error(t, err, "Unexpected resolved path: %v for: %v, fs: %v", resolved, path, fs.Name())
-		} else {
-			assert.Equal(t, fs.Join(user.GetHomeDir(), "/test/sub"), resolved)
-		}
+		assert.NoError(t, err)
+		assert.Equal(t, fs.Join(user.GetHomeDir(), "/test/sub"), resolved)
 		path = "../../../test/../sub"
 		resolved, err = fs.ResolvePath(filepath.ToSlash(path))
-		if vfs.IsLocalOsFs(fs) {
-			assert.Error(t, err, "Unexpected resolved path: %v for: %v, fs: %v", resolved, path, fs.Name())
-		} else {
-			assert.Equal(t, fs.Join(user.GetHomeDir(), "/sub"), resolved)
-		}
+		assert.NoError(t, err)
+		assert.Equal(t, fs.Join(user.GetHomeDir(), "/sub"), resolved)
 	}
 	err = os.RemoveAll(user.GetHomeDir())
 	assert.NoError(t, err)

+ 8 - 2
internal/util/util.go

@@ -165,6 +165,12 @@ func RemoveDuplicates(obj []string, trim bool) []string {
 
 // IsNameValid validates that a name/username contains only safe characters.
 func IsNameValid(name string) bool {
+	if name == "" {
+		return false
+	}
+	if len(name) > 255 {
+		return false
+	}
 	for _, r := range name {
 		if unicode.IsControl(r) {
 			return false
@@ -173,7 +179,7 @@ func IsNameValid(name string) bool {
 		switch r {
 		case '/', '\\':
 			return false
-		case ':':
+		case ':', '*', '?', '"', '<', '>', '|':
 			return false
 		}
 	}
@@ -542,7 +548,7 @@ func CleanPath(p string) string {
 // CleanPathWithBase returns a clean POSIX (/) absolute path to work with.
 // The specified base will be used if the provided path is not absolute
 func CleanPathWithBase(base, p string) string {
-	p = filepath.ToSlash(p)
+	p = strings.ReplaceAll(p, "\\", "/")
 	if !path.IsAbs(p) {
 		p = path.Join(base, p)
 	}

+ 3 - 2
internal/vfs/azblobfs.go

@@ -668,9 +668,10 @@ func (*AzureBlobFs) HasVirtualFolders() bool {
 
 // ResolvePath returns the matching filesystem path for the specified sftp path
 func (fs *AzureBlobFs) ResolvePath(virtualPath string) (string, error) {
-	virtualPath = strings.ReplaceAll(virtualPath, "\\", "/")
 	if fs.mountPath != "" {
-		virtualPath = strings.TrimPrefix(virtualPath, fs.mountPath)
+		if after, found := strings.CutPrefix(virtualPath, fs.mountPath); found {
+			virtualPath = after
+		}
 	}
 	virtualPath = path.Clean("/" + virtualPath)
 	return fs.Join(fs.config.KeyPrefix, strings.TrimPrefix(virtualPath, "/")), nil

+ 3 - 2
internal/vfs/gcsfs.go

@@ -637,9 +637,10 @@ func (*GCSFs) HasVirtualFolders() bool {
 
 // ResolvePath returns the matching filesystem path for the specified virtual path
 func (fs *GCSFs) ResolvePath(virtualPath string) (string, error) {
-	virtualPath = strings.ReplaceAll(virtualPath, "\\", "/")
 	if fs.mountPath != "" {
-		virtualPath = strings.TrimPrefix(virtualPath, fs.mountPath)
+		if after, found := strings.CutPrefix(virtualPath, fs.mountPath); found {
+			virtualPath = after
+		}
 	}
 	virtualPath = path.Clean("/" + virtualPath)
 	return fs.Join(fs.config.KeyPrefix, strings.TrimPrefix(virtualPath, "/")), nil

+ 4 - 4
internal/vfs/httpfs.go

@@ -637,12 +637,12 @@ func (*HTTPFs) HasVirtualFolders() bool {
 
 // ResolvePath returns the matching filesystem path for the specified virtual path
 func (fs *HTTPFs) ResolvePath(virtualPath string) (string, error) {
-	virtualPath = strings.ReplaceAll(virtualPath, "\\", "/")
 	if fs.mountPath != "" {
-		virtualPath = strings.TrimPrefix(virtualPath, fs.mountPath)
+		if after, found := strings.CutPrefix(virtualPath, fs.mountPath); found {
+			virtualPath = after
+		}
 	}
-	virtualPath = path.Clean("/" + virtualPath)
-	return virtualPath, nil
+	return path.Clean("/" + virtualPath), nil
 }
 
 // GetMimeType returns the content type

+ 11 - 5
internal/vfs/osfs.go

@@ -357,12 +357,16 @@ func (fs *OsFs) GetRelativePath(name string) string {
 	}
 	rel, err := filepath.Rel(fs.rootDir, filepath.Clean(name))
 	if err != nil {
-		return ""
+		return virtualPath
 	}
-	if rel == "." || strings.HasPrefix(rel, "..") {
+	rel = filepath.ToSlash(rel)
+	if rel == ".." || strings.HasPrefix(rel, "../") {
+		return virtualPath
+	}
+	if rel == "." {
 		rel = ""
 	}
-	return path.Join(virtualPath, filepath.ToSlash(rel))
+	return path.Join(virtualPath, rel)
 }
 
 // Walk walks the file tree rooted at root, calling walkFn for each file or
@@ -378,13 +382,15 @@ func (*OsFs) Join(elem ...string) string {
 
 // ResolvePath returns the matching filesystem path for the specified sftp path
 func (fs *OsFs) ResolvePath(virtualPath string) (string, error) {
-	virtualPath = strings.ReplaceAll(virtualPath, "\\", "/")
 	if !filepath.IsAbs(fs.rootDir) {
 		return "", fmt.Errorf("invalid root path %q", fs.rootDir)
 	}
 	if fs.mountPath != "" {
-		virtualPath = strings.TrimPrefix(virtualPath, fs.mountPath)
+		if after, found := strings.CutPrefix(virtualPath, fs.mountPath); found {
+			virtualPath = after
+		}
 	}
+	virtualPath = path.Clean("/" + virtualPath)
 	r := filepath.Clean(filepath.Join(fs.rootDir, virtualPath))
 	p, err := filepath.EvalSymlinks(r)
 	if isInvalidNameError(err) {

+ 3 - 2
internal/vfs/s3fs.go

@@ -610,9 +610,10 @@ func (*S3Fs) HasVirtualFolders() bool {
 
 // ResolvePath returns the matching filesystem path for the specified virtual path
 func (fs *S3Fs) ResolvePath(virtualPath string) (string, error) {
-	virtualPath = strings.ReplaceAll(virtualPath, "\\", "/")
 	if fs.mountPath != "" {
-		virtualPath = strings.TrimPrefix(virtualPath, fs.mountPath)
+		if after, found := strings.CutPrefix(virtualPath, fs.mountPath); found {
+			virtualPath = after
+		}
 	}
 	virtualPath = path.Clean("/" + virtualPath)
 	return fs.Join(fs.config.KeyPrefix, strings.TrimPrefix(virtualPath, "/")), nil

+ 21 - 8
internal/vfs/sftpfs.go

@@ -541,7 +541,7 @@ func (fs *SFTPFs) Readlink(name string) (string, error) {
 	if err != nil {
 		return resolved, err
 	}
-	resolved = path.Clean(resolved)
+	resolved = path.Clean(strings.ReplaceAll(resolved, "\\", "/"))
 	if !path.IsAbs(resolved) {
 		// we assume that multiple links are not followed
 		resolved = path.Join(path.Dir(name), resolved)
@@ -683,13 +683,23 @@ func (fs *SFTPFs) GetRelativePath(name string) string {
 		rel = ""
 	}
 	if !path.IsAbs(rel) {
-		return "/" + rel
-	}
-	if fs.config.Prefix != "/" {
-		if !strings.HasPrefix(rel, fs.config.Prefix) {
+		// If we have a relative path we assume it is already relative to the virtual root
+		rel = "/" + rel
+	} else if fs.config.Prefix != "/" {
+		prefixDir := fs.config.Prefix
+		if !strings.HasSuffix(prefixDir, "/") {
+			prefixDir += "/"
+		}
+
+		if rel == fs.config.Prefix {
+			rel = "/"
+		} else if after, found := strings.CutPrefix(rel, prefixDir); found {
+			rel = path.Clean("/" + after)
+		} else {
+			// Absolute path outside of the configured prefix
+			fsLog(fs, logger.LevelWarn, "path %q is an absolute path outside %q", name, fs.config.Prefix)
 			rel = "/"
 		}
-		rel = path.Clean("/" + strings.TrimPrefix(rel, fs.config.Prefix))
 	}
 	if fs.mountPath != "" {
 		rel = path.Join(fs.mountPath, rel)
@@ -730,9 +740,10 @@ func (*SFTPFs) HasVirtualFolders() bool {
 
 // ResolvePath returns the matching filesystem path for the specified virtual path
 func (fs *SFTPFs) ResolvePath(virtualPath string) (string, error) {
-	virtualPath = strings.ReplaceAll(virtualPath, "\\", "/")
 	if fs.mountPath != "" {
-		virtualPath = strings.TrimPrefix(virtualPath, fs.mountPath)
+		if after, found := strings.CutPrefix(virtualPath, fs.mountPath); found {
+			virtualPath = after
+		}
 	}
 	virtualPath = path.Clean("/" + virtualPath)
 	fsPath := fs.Join(fs.config.Prefix, virtualPath)
@@ -781,6 +792,7 @@ func (fs *SFTPFs) RealPath(p string) (string, error) {
 	if err != nil {
 		return "", err
 	}
+	resolved = path.Clean(strings.ReplaceAll(resolved, "\\", "/"))
 	if fs.config.Prefix != "/" {
 		if err := fs.isSubDir(resolved); err != nil {
 			fsLog(fs, logger.LevelError, "Invalid real path resolution, original path %q resolved %q err: %v",
@@ -810,6 +822,7 @@ func (fs *SFTPFs) getRealPath(name string) (string, error) {
 		if err != nil {
 			return name, fmt.Errorf("unable to resolve link to %q: %w", name, err)
 		}
+		resolvedLink = strings.ReplaceAll(resolvedLink, "\\", "/")
 		resolvedLink = path.Clean(resolvedLink)
 		if path.IsAbs(resolvedLink) {
 			name = resolvedLink