Replace error handling package with emperror; add better reporting for errors escaping server root

This commit is contained in:
Dane Everitt
2020-11-08 13:52:20 -08:00
parent 0989c78d4b
commit be9d1a3986
55 changed files with 396 additions and 367 deletions

View File

@@ -2,9 +2,9 @@ package filesystem
import (
"context"
"emperror.dev/errors"
"fmt"
"github.com/karrick/godirwalk"
"github.com/pkg/errors"
"github.com/pterodactyl/wings/server/backup"
ignore "github.com/sabhiram/go-gitignore"
"os"
@@ -41,7 +41,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In
if e.IsSymlink() {
sp, err = fs.SafePath(p)
if err != nil {
if errors.Is(err, ErrBadPathResolution) {
if IsBadPathResolutionError(err) {
return godirwalk.SkipThis
}
@@ -65,7 +65,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In
},
})
return inc, errors.WithStack(err)
return inc, errors.WithStackIf(err)
}
// Compresses all of the files matching the given paths in the specified directory. This function
@@ -115,7 +115,7 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er
// use the resolved location for the rest of this function.
sp, err = fs.SafePath(p)
if err != nil {
if errors.Is(err, ErrBadPathResolution) {
if IsBadPathResolutionError(err) {
return godirwalk.SkipThis
}
@@ -141,7 +141,7 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er
d := path.Join(cleanedRootDir, fmt.Sprintf("archive-%s.tar.gz", strings.ReplaceAll(time.Now().Format(time.RFC3339), ":", "")))
if err := a.Create(d, context.Background()); err != nil {
return nil, errors.WithStack(err)
return nil, errors.WithStackIf(err)
}
f, err := os.Stat(d)

View File

@@ -4,9 +4,9 @@ import (
"archive/tar"
"archive/zip"
"compress/gzip"
"emperror.dev/errors"
"fmt"
"github.com/mholt/archiver/v3"
"github.com/pkg/errors"
"os"
"path/filepath"
"reflect"
@@ -47,10 +47,10 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b
return false, ErrUnknownArchiveFormat
}
return false, errors.WithStack(err)
return false, errors.WithStackIf(err)
}
return true, errors.WithStack(err)
return true, errors.WithStackIf(err)
}
// Decompress a file in a given directory by using the archiver tool to infer the file
@@ -60,12 +60,12 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b
func (fs *Filesystem) DecompressFile(dir string, file string) error {
source, err := fs.SafePath(filepath.Join(dir, file))
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
// Make sure the file exists basically.
if _, err := os.Stat(source); err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
// Walk over all of the files spinning up an additional go-routine for each file we've encountered
@@ -93,17 +93,17 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error {
p, err := fs.SafePath(filepath.Join(dir, name))
if err != nil {
return errors.Wrap(err, "failed to generate a safe path to server file")
return errors.WrapIf(err, "failed to generate a safe path to server file")
}
return errors.Wrap(fs.Writefile(p, f), "could not extract file from archive")
return errors.WrapIf(fs.Writefile(p, f), "could not extract file from archive")
})
if err != nil {
if strings.HasPrefix(err.Error(), "format ") {
return errors.WithStack(ErrUnknownArchiveFormat)
return errors.WithStackIf(ErrUnknownArchiveFormat)
}
return errors.WithStack(err)
return errors.WithStackIf(err)
}
return nil

View File

@@ -1,9 +1,9 @@
package filesystem
import (
"emperror.dev/errors"
"github.com/apex/log"
"github.com/karrick/godirwalk"
"github.com/pkg/errors"
"sync"
"sync/atomic"
"syscall"
@@ -153,7 +153,7 @@ func (fs *Filesystem) updateCachedDiskUsage() (int64, error) {
func (fs *Filesystem) DirectorySize(dir string) (int64, error) {
d, err := fs.SafePath(dir)
if err != nil {
return 0, errors.WithStack(err)
return 0, errors.WithStackIf(err)
}
var size int64
@@ -167,7 +167,7 @@ func (fs *Filesystem) DirectorySize(dir string) (int64, error) {
// it. Otherwise, allow it to continue.
if e.IsSymlink() {
if _, err := fs.SafePath(p); err != nil {
if errors.Is(err, ErrBadPathResolution) {
if IsBadPathResolutionError(err) {
return godirwalk.SkipThis
}
@@ -184,7 +184,7 @@ func (fs *Filesystem) DirectorySize(dir string) (int64, error) {
},
})
return size, errors.WithStack(err)
return size, errors.WithStackIf(err)
}
// Helper function to determine if a server has space available for a file of a given size.

View File

@@ -1,16 +1,43 @@
package filesystem
import (
"emperror.dev/errors"
"fmt"
"github.com/apex/log"
"github.com/pkg/errors"
"os"
"path/filepath"
)
var ErrIsDirectory = errors.New("filesystem: is a directory")
var ErrNotEnoughDiskSpace = errors.New("filesystem: not enough disk space")
var ErrBadPathResolution = errors.New("filesystem: invalid path resolution")
var ErrUnknownArchiveFormat = errors.New("filesystem: unknown archive format")
var ErrIsDirectory = errors.Sentinel("filesystem: is a directory")
var ErrNotEnoughDiskSpace = errors.Sentinel("filesystem: not enough disk space")
var ErrUnknownArchiveFormat = errors.Sentinel("filesystem: unknown archive format")
type BadPathResolutionError struct {
path string
resolved string
}
// Returns the specific error for a bad path resolution.
func (b *BadPathResolutionError) Error() string {
r := b.resolved
if r == "" {
r = "<empty>"
}
return fmt.Sprintf("filesystem: server path [%s] resolves to a location outside the server root: %s", b.path, r)
}
// Returns a new BadPathResolution error.
func NewBadPathResolution(path string, resolved string) *BadPathResolutionError {
return &BadPathResolutionError{path, resolved}
}
// Determines if the given error is a bad path resolution error.
func IsBadPathResolutionError(err error) bool {
if _, ok := err.(*BadPathResolutionError); ok {
return true
}
return false
}
// Generates an error logger instance with some basic information.
func (fs *Filesystem) error(err error) *log.Entry {
@@ -23,8 +50,8 @@ func (fs *Filesystem) error(err error) *log.Entry {
// directory, otherwise return nil. Returning this error for a file will stop the walking
// for the remainder of the directory. This is assuming an os.FileInfo struct was even returned.
func (fs *Filesystem) handleWalkerError(err error, f os.FileInfo) error {
if !errors.Is(err, ErrBadPathResolution) {
return err
if !IsBadPathResolutionError(err) {
return errors.WithStackIf(err)
}
if f != nil && f.IsDir() {
@@ -32,4 +59,4 @@ func (fs *Filesystem) handleWalkerError(err error, f os.FileInfo) error {
}
return nil
}
}

View File

@@ -2,9 +2,9 @@ package filesystem
import (
"bufio"
"emperror.dev/errors"
"github.com/gabriel-vasile/mimetype"
"github.com/karrick/godirwalk"
"github.com/pkg/errors"
"github.com/pterodactyl/wings/config"
"github.com/pterodactyl/wings/system"
"io"
@@ -80,7 +80,7 @@ func (fs *Filesystem) Readfile(p string, w io.Writer) error {
func (fs *Filesystem) Writefile(p string, r io.Reader) error {
cleaned, err := fs.SafePath(p)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
var currentSize int64
@@ -88,15 +88,15 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
// to it and an empty file. We'll then write to it later on after this completes.
if stat, err := os.Stat(cleaned); err != nil {
if !os.IsNotExist(err) {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
if err := os.MkdirAll(filepath.Dir(cleaned), 0755); err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
if err := fs.Chown(filepath.Dir(cleaned)); err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
} else {
if stat.IsDir() {
@@ -119,7 +119,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
// truncate the existing file.
file, err := o.open(cleaned, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
defer file.Close()
@@ -138,7 +138,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
func (fs *Filesystem) CreateDirectory(name string, p string) error {
cleaned, err := fs.SafePath(path.Join(p, name))
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
return os.MkdirAll(cleaned, 0755)
@@ -148,12 +148,12 @@ func (fs *Filesystem) CreateDirectory(name string, p string) error {
func (fs *Filesystem) Rename(from string, to string) error {
cleanedFrom, err := fs.SafePath(from)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
cleanedTo, err := fs.SafePath(to)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
// If the target file or directory already exists the rename function will fail, so just
@@ -171,7 +171,7 @@ func (fs *Filesystem) Rename(from string, to string) error {
// we're not at the root directory level.
if d != fs.Path() {
if mkerr := os.MkdirAll(d, 0755); mkerr != nil {
return errors.Wrap(mkerr, "failed to create directory structure for file rename")
return errors.WrapIf(mkerr, "failed to create directory structure for file rename")
}
}
@@ -185,7 +185,7 @@ func (fs *Filesystem) Rename(from string, to string) error {
func (fs *Filesystem) Chown(path string) error {
cleaned, err := fs.SafePath(path)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
if fs.isTest {
@@ -197,7 +197,7 @@ func (fs *Filesystem) Chown(path string) error {
// Start by just chowning the initial path that we received.
if err := os.Chown(cleaned, uid, gid); err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
// If this is not a directory we can now return from the function, there is nothing
@@ -268,12 +268,12 @@ func (fs *Filesystem) findCopySuffix(dir string, name string, extension string)
func (fs *Filesystem) Copy(p string) error {
cleaned, err := fs.SafePath(p)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
s, err := os.Stat(cleaned)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
} else if s.IsDir() || !s.Mode().IsRegular() {
// If this is a directory or not a regular file, just throw a not-exist error
// since anything calling this function should understand what that means.
@@ -300,7 +300,7 @@ func (fs *Filesystem) Copy(p string) error {
source, err := os.Open(cleaned)
if err != nil {
return errors.WithStack(err)
return errors.WithStackIf(err)
}
defer source.Close()
@@ -324,7 +324,7 @@ func (fs *Filesystem) Delete(p string) error {
// exists within the data directory.
resolved := fs.unsafeFilePath(p)
if !fs.unsafeIsInDataDirectory(resolved) {
return ErrBadPathResolution
return NewBadPathResolution(p, resolved)
}
// Block any whoopsies.

View File

@@ -134,22 +134,22 @@ func TestFilesystem_SafePath(t *testing.T) {
g.It("blocks access to files outside the root directory", func() {
p, err := fs.SafePath("../test.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
g.Assert(p).Equal("")
p, err = fs.SafePath("/../test.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
g.Assert(p).Equal("")
p, err = fs.SafePath("./foo/../../test.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
g.Assert(p).Equal("")
p, err = fs.SafePath("..")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
g.Assert(p).Equal("")
})
})
@@ -185,7 +185,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
err := fs.Readfile("symlinked.txt", &b)
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
})
@@ -195,7 +195,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
err := fs.Writefile("symlinked.txt", r)
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("cannot write a file to a directory symlinked outside the root", func() {
@@ -203,7 +203,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
err := fs.Writefile("external_dir/foo.txt", r)
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
})
@@ -211,19 +211,19 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
g.It("cannot create a directory outside the root", func() {
err := fs.CreateDirectory("my_dir", "external_dir")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("cannot create a nested directory outside the root", func() {
err := fs.CreateDirectory("my/nested/dir", "external_dir/foo/bar")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("cannot create a nested directory outside the root", func() {
err := fs.CreateDirectory("my/nested/dir", "external_dir/server")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
})
@@ -231,13 +231,13 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
g.It("cannot rename a file symlinked outside the directory root", func() {
err := fs.Rename("symlinked.txt", "foo.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("cannot rename a symlinked directory outside the root", func() {
err := fs.Rename("external_dir", "foo")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("cannot rename a file to a location outside the directory root", func() {
@@ -245,7 +245,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
err := fs.Rename("my_file.txt", "external_dir/my_file.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
})
@@ -253,13 +253,13 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
g.It("cannot chown a file symlinked outside the directory root", func() {
err := fs.Chown("symlinked.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("cannot chown a directory symlinked outside the directory root", func() {
err := fs.Chown("external_dir")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
})
@@ -267,7 +267,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) {
g.It("cannot copy a file symlinked outside the directory root", func() {
err := fs.Copy("symlinked.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
})
@@ -325,7 +325,7 @@ func TestFilesystem_Readfile(t *testing.T) {
err = fs.Readfile("/../test.txt", buf)
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.AfterEach(func() {
@@ -386,7 +386,7 @@ func TestFilesystem_Writefile(t *testing.T) {
err := fs.Writefile("/some/../foo/../../test.txt", r)
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("cannot write a file that exceeds the disk limits", func() {
@@ -477,7 +477,7 @@ func TestFilesystem_CreateDirectory(t *testing.T) {
g.It("should not allow the creation of directories outside the root", func() {
err := fs.CreateDirectory("test", "e/../../something")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("should not increment the disk usage", func() {
@@ -527,7 +527,7 @@ func TestFilesystem_Rename(t *testing.T) {
g.It("does not allow renaming to a location outside the root", func() {
err := fs.Rename("source.txt", "../target.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("does not allow renaming from a location outside the root", func() {
@@ -535,7 +535,7 @@ func TestFilesystem_Rename(t *testing.T) {
err = fs.Rename("/../ext-source.txt", "target.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("allows a file to be renamed", func() {
@@ -613,7 +613,7 @@ func TestFilesystem_Copy(t *testing.T) {
err = fs.Copy("../ext-source.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("should return an error if the source directory is outside the root", func() {
@@ -625,11 +625,11 @@ func TestFilesystem_Copy(t *testing.T) {
err = fs.Copy("../nested/in/dir/ext-source.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
err = fs.Copy("nested/in/../../../nested/in/dir/ext-source.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("should return an error if the source is a directory", func() {
@@ -721,7 +721,7 @@ func TestFilesystem_Delete(t *testing.T) {
err = fs.Delete("../ext-source.txt")
g.Assert(err).IsNotNil()
g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue()
g.Assert(IsBadPathResolutionError(err)).IsTrue()
})
g.It("does not allow the deletion of the root directory", func() {

View File

@@ -2,6 +2,7 @@ package filesystem
import (
"context"
"emperror.dev/errors"
"golang.org/x/sync/errgroup"
"os"
"path/filepath"
@@ -23,9 +24,9 @@ func (fs *Filesystem) SafePath(p string) (string, error) {
// At the same time, evaluate the symlink status and determine where this file or folder
// is truly pointing to.
p, err := filepath.EvalSymlinks(r)
ep, err := filepath.EvalSymlinks(r)
if err != nil && !os.IsNotExist(err) {
return "", err
return "", errors.WithStackIf(err)
} 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.
@@ -53,7 +54,7 @@ func (fs *Filesystem) SafePath(p string) (string, error) {
// attempt going on, and we should NOT resolve this path for them.
if nonExistentPathResolution != "" {
if !fs.unsafeIsInDataDirectory(nonExistentPathResolution) {
return "", ErrBadPathResolution
return "", NewBadPathResolution(p, nonExistentPathResolution)
}
// If the nonExistentPathResolution variable is not empty then the initial path requested
@@ -66,11 +67,11 @@ func (fs *Filesystem) SafePath(p string) (string, error) {
// 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(p) {
return p, nil
if fs.unsafeIsInDataDirectory(ep) {
return ep, nil
}
return "", ErrBadPathResolution
return "", NewBadPathResolution(p, r)
}
// Generate a path to the file by cleaning it up and appending the root server path to it. This

View File

@@ -1,6 +1,7 @@
package filesystem
import (
"emperror.dev/errors"
"encoding/json"
"github.com/gabriel-vasile/mimetype"
"os"
@@ -50,14 +51,14 @@ func (fs *Filesystem) Stat(p string) (*Stat, error) {
func (fs *Filesystem) unsafeStat(p string) (*Stat, error) {
s, err := os.Stat(p)
if err != nil {
return nil, err
return nil, errors.WithStackIf(err)
}
var m *mimetype.MIME
if !s.IsDir() {
m, err = mimetype.DetectFile(p)
if err != nil {
return nil, err
return nil, errors.WithStackIf(err)
}
}