server(filesystem): SafePath tweaks

This commit is contained in:
Matthew Penner 2023-02-07 19:14:35 -07:00
parent 519d38f238
commit dac9685298
No known key found for this signature in database
3 changed files with 45 additions and 41 deletions

View File

@ -165,7 +165,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
// Adjust the disk usage to account for the old size and the new size of the file.
fs.addDisk(sz - currentSize)
return fs.Chown(cleaned)
return fs.unsafeChown(cleaned)
}
// Creates a new directory (name) at a specified path (p) for the server.
@ -223,7 +223,12 @@ func (fs *Filesystem) Chown(path string) error {
if err != nil {
return err
}
return fs.unsafeChown(cleaned)
}
// unsafeChown chowns the given path, without checking if the path is safe. This should only be used
// when the path has already been checked.
func (fs *Filesystem) unsafeChown(path string) error {
if fs.isTest {
return nil
}
@ -232,19 +237,19 @@ func (fs *Filesystem) Chown(path string) error {
gid := config.Get().System.User.Gid
// Start by just chowning the initial path that we received.
if err := os.Chown(cleaned, uid, gid); err != nil {
if err := os.Chown(path, uid, gid); err != nil {
return errors.Wrap(err, "server/filesystem: chown: failed to chown path")
}
// If this is not a directory we can now return from the function, there is nothing
// left that we need to do.
if st, err := os.Stat(cleaned); err != nil || !st.IsDir() {
if st, err := os.Stat(path); err != nil || !st.IsDir() {
return nil
}
// If this was a directory, begin walking over its contents recursively and ensure that all
// of the subfiles and directories get their permissions updated as well.
err = godirwalk.Walk(cleaned, &godirwalk.Options{
err := godirwalk.Walk(path, &godirwalk.Options{
Unsorted: true,
Callback: func(p string, e *godirwalk.Dirent) error {
// Do not attempt to chown a symlink. Go's os.Chown function will affect the symlink
@ -261,7 +266,6 @@ func (fs *Filesystem) Chown(path string) error {
return os.Chown(p, uid, gid)
},
})
return errors.Wrap(err, "server/filesystem: chown: failed to chown during walk function")
}

View File

@ -2,6 +2,7 @@ package filesystem
import (
"context"
iofs "io/fs"
"os"
"path/filepath"
"strings"
@ -33,8 +34,6 @@ func (fs *Filesystem) IsIgnored(paths ...string) error {
// This logic is actually copied over from the SFTP server code. Ideally that eventually
// either gets ported into this application, or is able to make use of this package.
func (fs *Filesystem) SafePath(p string) (string, error) {
var nonExistentPathResolution string
// Start with a cleaned up path before checking the more complex bits.
r := fs.unsafeFilePath(p)
@ -44,47 +43,24 @@ func (fs *Filesystem) SafePath(p string) (string, error) {
if err != nil && !os.IsNotExist(err) {
return "", errors.Wrap(err, "server/filesystem: failed to evaluate symlink")
} else if os.IsNotExist(err) {
// The requested directory doesn't exist, so at this point we need to iterate up the
// path chain until we hit a directory that _does_ exist and can be validated.
parts := strings.Split(filepath.Dir(r), "/")
var try string
// Range over all of the path parts and form directory pathings from the end
// moving up until we have a valid resolution or we run out of paths to try.
for k := range parts {
try = strings.Join(parts[:(len(parts)-k)], "/")
if !fs.unsafeIsInDataDirectory(try) {
break
// The target of one of the symlinks (EvalSymlinks is recursive) does not exist.
// So we get what target path does not exist and check if it's within the data
// directory. If it is, we return the original path, otherwise we return an error.
pErr, ok := err.(*iofs.PathError)
if !ok {
return "", errors.Wrap(err, "server/filesystem: failed to evaluate symlink")
}
t, err := filepath.EvalSymlinks(try)
if err == nil {
nonExistentPathResolution = t
break
}
}
}
// If the new path doesn't start with their root directory there is clearly an escape
// attempt going on, and we should NOT resolve this path for them.
if nonExistentPathResolution != "" {
if !fs.unsafeIsInDataDirectory(nonExistentPathResolution) {
return "", NewBadPathResolution(p, nonExistentPathResolution)
}
// If the nonExistentPathResolution variable is not empty then the initial path requested
// did not exist and we looped through the pathway until we found a match. At this point
// we've confirmed the first matched pathway exists in the root server directory, so we
// can go ahead and just return the path that was requested initially.
return r, nil
ep = pErr.Path
}
// If the requested directory from EvalSymlinks begins with the server root directory go
// ahead and return it. If not we'll return an error which will block any further action
// on the file.
if fs.unsafeIsInDataDirectory(ep) {
return ep, nil
// Returning the original path here instead of the resolved path ensures that
// whatever the user is trying to do will work as expected. If we returned the
// resolved path, the user would be unable to know that it is in fact a symlink.
return r, nil
}
return "", NewBadPathResolution(p, r)

View File

@ -115,6 +115,14 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
panic(err)
}
if err := os.Symlink(filepath.Join(rfs.root, "malicious_does_not_exist.txt"), filepath.Join(rfs.root, "/server/symlinked_does_not_exist.txt")); err != nil {
panic(err)
}
if err := os.Symlink(filepath.Join(rfs.root, "/server/symlinked_does_not_exist.txt"), filepath.Join(rfs.root, "/server/symlinked_does_not_exist2.txt")); err != nil {
panic(err)
}
if err := os.Symlink(filepath.Join(rfs.root, "/malicious_dir"), filepath.Join(rfs.root, "/server/external_dir")); err != nil {
panic(err)
}
@ -128,6 +136,22 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue()
})
g.It("cannot write to a non-existent file symlinked outside the root", func() {
r := bytes.NewReader([]byte("testing what the fuck"))
err := fs.Writefile("symlinked_does_not_exist.txt", r)
g.Assert(err).IsNotNil()
g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue()
})
g.It("cannot write to chained symlinks with target that does not exist outside the root", func() {
r := bytes.NewReader([]byte("testing what the fuck"))
err := fs.Writefile("symlinked_does_not_exist2.txt", r)
g.Assert(err).IsNotNil()
g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue()
})
g.It("cannot write a file to a directory symlinked outside the root", func() {
r := bytes.NewReader([]byte("testing"))