Compare commits
4 Commits
develop
...
v1.7.4-hot
Author | SHA1 | Date | |
---|---|---|---|
|
fb65487ed8 | ||
|
dcbc59790d | ||
|
00451b38db | ||
|
9f6548eaa8 |
|
@ -1,5 +1,13 @@
|
|||
# Changelog
|
||||
|
||||
## v1.7.4
|
||||
### Fixed
|
||||
* CVE-2023-25168
|
||||
|
||||
## v1.7.3
|
||||
### Fixed
|
||||
* CVE-2023-25152
|
||||
|
||||
## v1.7.2
|
||||
### Fixed
|
||||
* The S3 backup driver now supports Cloudflare R2
|
||||
|
|
|
@ -159,7 +159,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.
|
||||
|
@ -217,7 +217,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
|
||||
}
|
||||
|
@ -226,19 +231,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
|
||||
|
@ -255,7 +260,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")
|
||||
}
|
||||
|
||||
|
@ -377,10 +381,9 @@ func (fs *Filesystem) TruncateRootDirectory() error {
|
|||
// Delete removes a file or folder from the system. Prevents the user from
|
||||
// accidentally (or maliciously) removing their root server data directory.
|
||||
func (fs *Filesystem) Delete(p string) error {
|
||||
wg := sync.WaitGroup{}
|
||||
// This is one of the few (only?) places in the codebase where we're explicitly not using
|
||||
// the SafePath functionality when working with user provided input. If we did, you would
|
||||
// not be able to delete a file that is a symlink pointing to a location outside of the data
|
||||
// not be able to delete a file that is a symlink pointing to a location outside the data
|
||||
// directory.
|
||||
//
|
||||
// We also want to avoid resolving a symlink that points _within_ the data directory and thus
|
||||
|
@ -397,25 +400,65 @@ func (fs *Filesystem) Delete(p string) error {
|
|||
return errors.New("cannot delete root server directory")
|
||||
}
|
||||
|
||||
if st, err := os.Lstat(resolved); err != nil {
|
||||
st, err := os.Lstat(resolved)
|
||||
if err != nil {
|
||||
if !os.IsNotExist(err) {
|
||||
fs.error(err).Warn("error while attempting to stat file before deletion")
|
||||
return err
|
||||
}
|
||||
} else {
|
||||
if !st.IsDir() {
|
||||
fs.addDisk(-st.Size())
|
||||
} else {
|
||||
wg.Add(1)
|
||||
go func(wg *sync.WaitGroup, st os.FileInfo, resolved string) {
|
||||
defer wg.Done()
|
||||
if s, err := fs.DirectorySize(resolved); err == nil {
|
||||
fs.addDisk(-s)
|
||||
|
||||
// The following logic is used to handle a case where a user attempts to
|
||||
// delete a file that does not exist through a directory symlink.
|
||||
// We don't want to reveal that the file does not exist, so we validate
|
||||
// the path of the symlink and return a bad path error if it is invalid.
|
||||
|
||||
// The requested file or 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(resolved), "/")
|
||||
|
||||
// Range over all the path parts and form directory paths 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
|
||||
}
|
||||
}(&wg, st, resolved)
|
||||
|
||||
t, err := filepath.EvalSymlinks(try)
|
||||
if err == nil {
|
||||
if !fs.unsafeIsInDataDirectory(t) {
|
||||
return NewBadPathResolution(p, t)
|
||||
}
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
// Always return early if the file does not exist.
|
||||
return nil
|
||||
}
|
||||
|
||||
// If the file is not a symlink, we need to check that it is not within a
|
||||
// symlinked directory that points outside the data directory.
|
||||
if st.Mode()&os.ModeSymlink == 0 {
|
||||
ep, err := filepath.EvalSymlinks(resolved)
|
||||
if err != nil {
|
||||
if !os.IsNotExist(err) {
|
||||
return err
|
||||
}
|
||||
} else if !fs.unsafeIsInDataDirectory(ep) {
|
||||
return NewBadPathResolution(p, ep)
|
||||
}
|
||||
}
|
||||
|
||||
if st.IsDir() {
|
||||
if s, err := fs.DirectorySize(resolved); err == nil {
|
||||
fs.addDisk(-s)
|
||||
}
|
||||
} else {
|
||||
fs.addDisk(-st.Size())
|
||||
}
|
||||
|
||||
return os.RemoveAll(resolved)
|
||||
}
|
||||
|
|
|
@ -508,6 +508,80 @@ func TestFilesystem_Delete(t *testing.T) {
|
|||
}
|
||||
})
|
||||
|
||||
g.It("deletes a symlink but not it's target within the root directory", func() {
|
||||
// Symlink to a file inside the root directory.
|
||||
err := os.Symlink(filepath.Join(rfs.root, "server/source.txt"), filepath.Join(rfs.root, "server/symlink.txt"))
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Delete the symlink itself.
|
||||
err = fs.Delete("symlink.txt")
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Ensure the symlink was deleted.
|
||||
_, err = os.Lstat(filepath.Join(rfs.root, "server/symlink.txt"))
|
||||
g.Assert(err).IsNotNil()
|
||||
|
||||
// Ensure the symlink target still exists.
|
||||
_, err = os.Lstat(filepath.Join(rfs.root, "server/source.txt"))
|
||||
g.Assert(err).IsNil()
|
||||
})
|
||||
|
||||
g.It("does not delete files symlinked outside of the root directory", func() {
|
||||
// Create a file outside the root directory.
|
||||
err := rfs.CreateServerFileFromString("/../source.txt", "test content")
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Create a symlink to the file outside the root directory.
|
||||
err = os.Symlink(filepath.Join(rfs.root, "source.txt"), filepath.Join(rfs.root, "/server/symlink.txt"))
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Delete the symlink. (This should pass as we will delete the symlink itself, not it's target)
|
||||
err = fs.Delete("symlink.txt")
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Ensure the file outside the root directory still exists.
|
||||
_, err = os.Lstat(filepath.Join(rfs.root, "source.txt"))
|
||||
g.Assert(err).IsNil()
|
||||
})
|
||||
|
||||
g.It("does not delete files symlinked through a directory outside of the root directory", func() {
|
||||
// Create a directory outside the root directory.
|
||||
err := os.Mkdir(filepath.Join(rfs.root, "foo"), 0o755)
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Create a file inside the directory that is outside the root.
|
||||
err = rfs.CreateServerFileFromString("/../foo/source.txt", "test content")
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Symlink the directory that is outside the root to a file inside the root.
|
||||
err = os.Symlink(filepath.Join(rfs.root, "foo"), filepath.Join(rfs.root, "server/symlink"))
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Delete a file inside the symlinked directory.
|
||||
err = fs.Delete("symlink/source.txt")
|
||||
g.Assert(err).IsNotNil()
|
||||
g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue()
|
||||
|
||||
// Ensure the file outside the root directory still exists.
|
||||
_, err = os.Lstat(filepath.Join(rfs.root, "foo/source.txt"))
|
||||
g.Assert(err).IsNil()
|
||||
})
|
||||
|
||||
g.It("returns an error when trying to delete a non-existent file symlinked through a directory outside of the root directory", func() {
|
||||
// Create a directory outside the root directory.
|
||||
err := os.Mkdir(filepath.Join(rfs.root, "foo2"), 0o755)
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Symlink the directory that is outside the root to a file inside the root.
|
||||
err = os.Symlink(filepath.Join(rfs.root, "foo2"), filepath.Join(rfs.root, "server/symlink"))
|
||||
g.Assert(err).IsNil()
|
||||
|
||||
// Delete a file inside the symlinked directory.
|
||||
err = fs.Delete("symlink/source.txt")
|
||||
g.Assert(err).IsNotNil()
|
||||
g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue()
|
||||
})
|
||||
|
||||
g.AfterEach(func() {
|
||||
rfs.reset()
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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"))
|
||||
|
||||
|
|
Loading…
Reference in New Issue
Block a user