make sure the usage of user provides paths is secure

cs3org · Jun 17, 2021 · b82b779 · b82b779
1 parent e7993cb
commit b82b779
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 4 deletions.
diff --git a/pkg/rhttp/datatx/utils/download/download.go b/pkg/rhttp/datatx/utils/download/download.go
@@ -32,6 +32,7 @@ import (
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/cs3org/reva/pkg/storage"
+	"github.com/cs3org/reva/pkg/utils"
 	"github.com/rs/zerolog"
 )
 
@@ -59,7 +60,7 @@ func GetOrHeadFile(w http.ResponseWriter, r *http.Request, fs storage.FS, spaceI
 			ref = &provider.Reference{
 				ResourceId: &provider.ResourceId{StorageId: parts[0], OpaqueId: parts[1]},
 				// ensure the relative path starts with '.'
-				Path: path.Clean(strings.TrimLeft(fn, "/")),
+				Path: utils.MakeRelativePath(fn),
 			}
 		} else {
 			sublog.Error().Str("space_id", spaceID).Str("path", fn).Msg("invalid reference")

diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go
@@ -263,7 +263,9 @@ func (fs *ocfs) toInternalPath(ctx context.Context, sp string) (ip string) {
 	if fs.c.EnableHome {
 		u := user.ContextMustGetUser(ctx)
 		layout := templates.WithUser(u, fs.c.UserLayout)
-		ip = filepath.Join(fs.c.DataDirectory, layout, "files", sp)
+		// The inner filepath.Join prevents the path from breaking out of
+		// <fs.c.DataDirectory>/<layout>/files/
+		ip = filepath.Join(fs.c.DataDirectory, layout, "files", filepath.Join("/", sp))
 	} else {
 		// trim all /
 		sp = strings.Trim(sp, "/")
@@ -290,7 +292,7 @@ func (fs *ocfs) toInternalPath(ctx context.Context, sp string) (ip string) {
 			ip = filepath.Join(fs.c.DataDirectory, layout, "files")
 		} else {
 			// parts = "<username>", "foo/bar.txt"
-			ip = filepath.Join(fs.c.DataDirectory, layout, "files", segments[1])
+			ip = filepath.Join(fs.c.DataDirectory, layout, "files", filepath.Join(segments[1]))
 		}
 
 	}

diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go
@@ -201,7 +201,7 @@ func isNotDir(err error) bool {
 
 // Child returns the child node with the given name
 func (n *Node) Child(ctx context.Context, name string) (*Node, error) {
-	link, err := os.Readlink(filepath.Join(n.InternalPath(), name))
+	link, err := os.Readlink(filepath.Join(n.InternalPath(), filepath.Join("/", name)))
 	if err != nil {
 		if os.IsNotExist(err) || isNotDir(err) {
 			c := &Node{

diff --git a/pkg/storage/utils/localfs/localfs.go b/pkg/storage/utils/localfs/localfs.go
@@ -161,6 +161,9 @@ func getUser(ctx context.Context) (*userpb.User, error) {
 }
 
 func (fs *localfs) wrap(ctx context.Context, p string) string {
+	// This is to prevent path traversal.
+	// With this p can't break out of its parent folder
+	p = path.Join("/", p)
 	var internal string
 	if !fs.conf.DisableHome {
 		layout, err := fs.GetHome(ctx)