diff --git a/go.mod b/go.mod index f174131..cecb97c 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/docker/go-metrics v0.0.1 // indirect github.com/docker/go-units v0.4.0 // indirect github.com/fatih/color v1.9.0 + github.com/franela/goblin v0.0.0-20200825194134-80c0062ed6cd github.com/frankban/quicktest v1.10.2 // indirect github.com/fsnotify/fsnotify v1.4.9 // indirect github.com/gabriel-vasile/mimetype v1.1.1 diff --git a/go.sum b/go.sum index 5733b1f..5c762cc 100644 --- a/go.sum +++ b/go.sum @@ -123,6 +123,8 @@ github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5Kwzbycv github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s= github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU= github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4= +github.com/franela/goblin v0.0.0-20200825194134-80c0062ed6cd h1:b/30UOB56Rhfe185ZfgvZT0/HOql0OzxuiNOxRKXRXc= +github.com/franela/goblin v0.0.0-20200825194134-80c0062ed6cd/go.mod h1:VzmDKDJVZI3aJmnRI9VjAn9nJ8qPPsN1fqzr9dqInIo= github.com/franela/goreq v0.0.0-20171204163338-bcd34c9993f8/go.mod h1:ZhphrRTfi2rbfLwlschooIH4+wKKDR4Pdxhh+TRoA20= github.com/frankban/quicktest v1.10.2 h1:19ARM85nVi4xH7xPXuc5eM/udya5ieh7b/Sv+d844Tk= github.com/frankban/quicktest v1.10.2/go.mod h1:K+q6oSqb0W0Ininfk863uOk1lMy69l/P6txr3mVT54s= diff --git a/router/router_server_files.go b/router/router_server_files.go index 2027dc6..712010a 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -1,7 +1,6 @@ package router import ( - "bufio" "context" "github.com/gin-gonic/gin" "github.com/pkg/errors" @@ -30,34 +29,12 @@ func getServerFileContents(c *gin.Context) { } p = "/" + strings.TrimLeft(p, "/") - cleaned, err := s.Filesystem().SafePath(p) + st, err := s.Filesystem().Stat(p) if err != nil { - c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ - "error": "The file requested could not be found.", - }) + TrackedServerError(err, s).AbortFilesystemError(c) return } - st, err := s.Filesystem().Stat(cleaned) - if err != nil { - TrackedServerError(err, s).AbortWithServerError(c) - return - } - - if st.Info.IsDir() { - c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ - "error": "The requested resource was not found on the system.", - }) - return - } - - f, err := os.Open(cleaned) - if err != nil { - TrackedServerError(err, s).AbortWithServerError(c) - return - } - defer f.Close() - c.Header("X-Mime-Type", st.Mimetype) c.Header("Content-Length", strconv.Itoa(int(st.Info.Size()))) @@ -68,7 +45,10 @@ func getServerFileContents(c *gin.Context) { c.Header("Content-Type", "application/octet-stream") } - bufio.NewReader(f).WriteTo(c.Writer) + if err := s.Filesystem().Readfile(p, c.Writer); err != nil { + TrackedServerError(err, s).AbortFilesystemError(c) + return + } } // Returns the contents of a directory for a server. diff --git a/server/archiver.go b/server/archiver.go index 9626b4b..f757bb1 100644 --- a/server/archiver.go +++ b/server/archiver.go @@ -62,9 +62,20 @@ func (a *Archiver) Archive() error { } for _, file := range fileInfo { - f, err := a.Server.Filesystem().SafeJoin(path, file) - if err != nil { - return err + f := filepath.Join(path, file.Name()) + // If the file is a symlink we cannot safely assume that the result of a filepath.Join() will be + // a safe destination. We need to check if the file is a symlink, and if so pass off to the SafePath + // function to resolve it to the final destination. + // + // ioutil.ReadDir() calls Lstat, so this will work correctly. If it did not call Lstat, but rather + // just did a normal Stat call, this would fail since that would be looking at the symlink destination + // and not the actual file in this listing. + if file.Mode()&os.ModeSymlink != 0 { + f, err = a.Server.Filesystem().SafePath(filepath.Join(path, file.Name())) + + if err != nil { + return err + } } files = append(files, f) diff --git a/server/filesystem/disk_space.go b/server/filesystem/disk_space.go index 4eb3273..4922b7a 100644 --- a/server/filesystem/disk_space.go +++ b/server/filesystem/disk_space.go @@ -199,7 +199,18 @@ func (fs *Filesystem) hasSpaceFor(size int64) error { // Updates the disk usage for the Filesystem instance. func (fs *Filesystem) addDisk(i int64) int64 { - size, _ := fs.DiskUsage(true) + var size = atomic.LoadInt64(&fs.diskUsed) + // Sorry go gods. This is ugly but the best approach I can come up with for right + // now without completely re-evaluating the logic we use for determining disk space. + // + // Normally I would just be using the atomic load right below, but I'm not sure about + // the scenarios where it is 0 because nothing has run that would trigger a disk size + // calculation? + // + // Perhaps that isn't even a concern for the sake of this? + if !fs.isTest { + size, _ = fs.DiskUsage(true) + } // If we're dropping below 0 somehow just cap it to 0. if (size + i) < 0 { diff --git a/server/filesystem/filesystem.go b/server/filesystem/filesystem.go index a98e0f9..a6b89a5 100644 --- a/server/filesystem/filesystem.go +++ b/server/filesystem/filesystem.go @@ -2,8 +2,6 @@ package filesystem import ( "bufio" - "bytes" - "fmt" "github.com/gabriel-vasile/mimetype" "github.com/karrick/godirwalk" "github.com/pkg/errors" @@ -33,6 +31,8 @@ type Filesystem struct { // The root data directory path for this Filesystem instance. root string + + isTest bool } // Creates a new Filesystem instance for a given server. @@ -53,18 +53,27 @@ func (fs *Filesystem) Path() string { // Reads a file on the system and returns it as a byte representation in a file // reader. This is not the most memory efficient usage since it will be reading the // entirety of the file into memory. -func (fs *Filesystem) Readfile(p string) (io.Reader, error) { +func (fs *Filesystem) Readfile(p string, w io.Writer) error { cleaned, err := fs.SafePath(p) if err != nil { - return nil, err + return err } - b, err := ioutil.ReadFile(cleaned) + if st, err := os.Stat(cleaned); err != nil { + return err + } else if st.IsDir() { + return ErrIsDirectory + } + + f, err := os.Open(cleaned) if err != nil { - return nil, err + return err } + defer f.Close() - return bytes.NewReader(b), nil + _, err = bufio.NewReader(f).WriteTo(w) + + return err } // Writes a file to the system. If the file does not already exist one will be created. @@ -161,7 +170,7 @@ func (fs *Filesystem) Rename(from string, to string) error { // Ensure that the directory we're moving into exists correctly on the system. Only do this if // we're not at the root directory level. if d != fs.Path() { - if mkerr := os.MkdirAll(d, 0644); mkerr != nil { + if mkerr := os.MkdirAll(d, 0755); mkerr != nil { return errors.Wrap(mkerr, "failed to create directory structure for file rename") } } @@ -179,6 +188,10 @@ func (fs *Filesystem) Chown(path string) error { return errors.WithStack(err) } + if fs.isTest { + return nil + } + uid := config.Get().System.User.Uid gid := config.Get().System.User.Gid @@ -214,6 +227,42 @@ func (fs *Filesystem) Chown(path string) error { }) } +// Begin looping up to 50 times to try and create a unique copy file name. This will take +// an input of "file.txt" and generate "file copy.txt". If that name is already taken, it will +// then try to write "file copy 2.txt" and so on, until reaching 50 loops. At that point we +// won't waste anymore time, just use the current timestamp and make that copy. +// +// Could probably make this more efficient by checking if there are any files matching the copy +// pattern, and trying to find the highest number and then incrementing it by one rather than +// looping endlessly. +func (fs *Filesystem) findCopySuffix(dir string, name string, extension string) (string, error) { + var i int + var suffix = " copy" + + for i = 0; i < 51; i++ { + if i > 0 { + suffix = " copy " + strconv.Itoa(i) + } + + n := name + suffix + extension + // If we stat the file and it does not exist that means we're good to create the copy. If it + // does exist, we'll just continue to the next loop and try again. + if _, err := fs.Stat(path.Join(dir, n)); err != nil { + if !os.IsNotExist(err) { + return "", err + } + + break + } + + if i == 50 { + suffix = "copy." + time.Now().Format(time.RFC3339) + } + } + + return name + suffix + extension, nil +} + // Copies a given file to the same location and appends a suffix to the file to indicate that // it has been copied. func (fs *Filesystem) Copy(p string) error { @@ -249,70 +298,21 @@ func (fs *Filesystem) Copy(p string) error { name = strings.TrimSuffix(name, ".tar") } - // Begin looping up to 50 times to try and create a unique copy file name. This will take - // an input of "file.txt" and generate "file copy.txt". If that name is already taken, it will - // then try to write "file copy 2.txt" and so on, until reaching 50 loops. At that point we - // won't waste anymore time, just use the current timestamp and make that copy. - // - // Could probably make this more efficient by checking if there are any files matching the copy - // pattern, and trying to find the highest number and then incrementing it by one rather than - // looping endlessly. - var i int - copySuffix := " copy" - for i = 0; i < 51; i++ { - if i > 0 { - copySuffix = " copy " + strconv.Itoa(i) - } - - tryName := fmt.Sprintf("%s%s%s", name, copySuffix, extension) - tryLocation, err := fs.SafePath(path.Join(relative, tryName)) - if err != nil { - return errors.WithStack(err) - } - - // If the file exists, continue to the next loop, otherwise we're good to start a copy. - if _, err := os.Stat(tryLocation); err != nil && !os.IsNotExist(err) { - return errors.WithStack(err) - } else if os.IsNotExist(err) { - break - } - - if i == 50 { - copySuffix = "." + time.Now().Format(time.RFC3339) - } - } - - finalPath, err := fs.SafePath(path.Join(relative, fmt.Sprintf("%s%s%s", name, copySuffix, extension))) - if err != nil { - return errors.WithStack(err) - } - source, err := os.Open(cleaned) if err != nil { return errors.WithStack(err) } defer source.Close() - dest, err := os.Create(finalPath) - if err != nil { - return errors.WithStack(err) - } - defer dest.Close() + n, err := fs.findCopySuffix(relative, name, extension) - buf := make([]byte, 1024*4) - if _, err := io.CopyBuffer(dest, source, buf); err != nil { - return errors.WithStack(err) - } - - // Once everything is done, increment the disk space used. - fs.addDisk(s.Size()) - - return nil + return fs.Writefile(path.Join(relative, n), source) } // Deletes 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 @@ -332,7 +332,7 @@ func (fs *Filesystem) Delete(p string) error { return errors.New("cannot delete root server directory") } - if st, err := os.Stat(resolved); err != nil { + if st, err := os.Lstat(resolved); err != nil { if !os.IsNotExist(err) { fs.error(err).Warn("error while attempting to stat file before deletion") } @@ -340,14 +340,18 @@ func (fs *Filesystem) Delete(p string) error { if !st.IsDir() { fs.addDisk(-st.Size()) } else { - go func(st os.FileInfo, resolved string) { + 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) } - }(st, resolved) + }(&wg, st, resolved) } } + wg.Wait() + return os.RemoveAll(resolved) } @@ -408,7 +412,11 @@ func (fs *Filesystem) ListDirectory(p string) ([]*Stat, error) { var m *mimetype.MIME var d = "inode/directory" if !f.IsDir() { - cleanedp, _ := fs.SafeJoin(cleaned, f) + cleanedp := filepath.Join(cleaned, f.Name()) + if f.Mode()&os.ModeSymlink != 0 { + cleanedp, _ = fs.SafePath(filepath.Join(cleaned, f.Name())) + } + if cleanedp != "" { m, _ = mimetype.DetectFile(filepath.Join(cleaned, f.Name())) } else { diff --git a/server/filesystem/filesystem_test.go b/server/filesystem/filesystem_test.go new file mode 100644 index 0000000..0ee8931 --- /dev/null +++ b/server/filesystem/filesystem_test.go @@ -0,0 +1,784 @@ +package filesystem + +import ( + "bytes" + "errors" + . "github.com/franela/goblin" + "github.com/pterodactyl/wings/config" + "io/ioutil" + "math/rand" + "os" + "path/filepath" + "testing" + "unicode/utf8" +) + +func NewFs() (*Filesystem, *rootFs) { + config.Set(&config.Configuration{ + AuthenticationToken: "abc", + System: config.SystemConfiguration{ + RootDirectory: "/server", + DiskCheckInterval: 150, + }, + }) + + tmpDir, err := ioutil.TempDir(os.TempDir(), "pterodactyl") + if err != nil { + panic(err) + } + // defer os.RemoveAll(tmpDir) + + rfs := rootFs{root: tmpDir} + + rfs.reset() + + fs := New(filepath.Join(tmpDir, "/server"), 0) + fs.isTest = true + + return fs, &rfs +} + +type rootFs struct { + root string +} + +func (rfs *rootFs) CreateServerFile(p string, c string) error { + f, err := os.Create(filepath.Join(rfs.root, "/server", p)) + + if err == nil { + f.Write([]byte(c)) + f.Close() + } + + return err +} + +func (rfs *rootFs) StatServerFile(p string) (os.FileInfo, error) { + return os.Stat(filepath.Join(rfs.root, "/server", p)) +} + +func (rfs *rootFs) reset() { + if err := os.RemoveAll(filepath.Join(rfs.root, "/server")); err != nil { + if !os.IsNotExist(err) { + panic(err) + } + } + + if err := os.Mkdir(filepath.Join(rfs.root, "/server"), 0755); err != nil { + panic(err) + } +} + +func TestFilesystem_Path(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + g.Describe("Path", func() { + g.It("returns the root path for the instance", func() { + g.Assert(fs.Path()).Equal(filepath.Join(rfs.root, "/server")) + }) + }) +} + +func TestFilesystem_SafePath(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + prefix := filepath.Join(rfs.root, "/server") + + g.Describe("SafePath", func() { + g.It("returns a cleaned path to a given file", func() { + p, err := fs.SafePath("test.txt") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix + "/test.txt") + + p, err = fs.SafePath("/test.txt") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix + "/test.txt") + + p, err = fs.SafePath("./test.txt") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix + "/test.txt") + + p, err = fs.SafePath("/foo/../test.txt") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix + "/test.txt") + + p, err = fs.SafePath("/foo/bar") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix + "/foo/bar") + }) + + g.It("handles root directory access", func() { + p, err := fs.SafePath("/") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix) + + p, err = fs.SafePath("") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix) + }) + + g.It("removes trailing slashes from paths", func() { + p, err := fs.SafePath("/foo/bar/") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix + "/foo/bar") + }) + + g.It("handles deeply nested directories that do not exist", func() { + p, err := fs.SafePath("/foo/bar/baz/quaz/../../ducks/testing.txt") + g.Assert(err).IsNil() + g.Assert(p).Equal(prefix + "/foo/bar/ducks/testing.txt") + }) + + 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(p).Equal("") + + p, err = fs.SafePath("/../test.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).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(p).Equal("") + + p, err = fs.SafePath("..") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + g.Assert(p).Equal("") + }) + }) +} + +// We test against accessing files outside the root directory in the tests, however it +// is still possible for someone to mess up and not properly use this safe path call. In +// order to truly confirm this, we'll try to pass in a symlinked malicious file to all of +// the calls and ensure they all fail with the same reason. +func TestFilesystem_Blocks_Symlinks(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + if err := rfs.CreateServerFile("/../malicious.txt", "external content"); err != nil { + panic(err) + } + + if err := os.Mkdir(filepath.Join(rfs.root, "/malicious_dir"), 0777); err != nil { + panic(err) + } + + if err := os.Symlink(filepath.Join(rfs.root, "malicious.txt"), filepath.Join(rfs.root, "/server/symlinked.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) + } + + g.Describe("Readfile", func() { + g.It("cannot read a file symlinked outside the root", func() { + b := bytes.Buffer{} + + err := fs.Readfile("symlinked.txt", &b) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + }) + + g.Describe("Writefile", func() { + g.It("cannot write to a file symlinked outside the root", func() { + r := bytes.NewReader([]byte("testing")) + + err := fs.Writefile("symlinked.txt", r) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + + g.It("cannot write a file to a directory symlinked outside the root", func() { + r := bytes.NewReader([]byte("testing")) + + err := fs.Writefile("external_dir/foo.txt", r) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + }) + + g.Describe("CreateDirectory", func() { + 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.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.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.Describe("Rename", func() { + 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.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.It("cannot rename a file to a location outside the directory root", func() { + rfs.CreateServerFile("my_file.txt", "internal content") + + err := fs.Rename("my_file.txt", "external_dir/my_file.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + }) + + g.Describe("Chown", func() { + 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.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.Describe("Copy", func() { + 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.Describe("Delete", func() { + g.It("deletes the symlinked file but leaves the source", func() { + err := fs.Delete("symlinked.txt") + g.Assert(err).IsNil() + + _, err = os.Stat(filepath.Join(rfs.root, "malicious.txt")) + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("symlinked.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + }) + }) + + rfs.reset() +} + +func TestFilesystem_Readfile(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + g.Describe("Readfile", func() { + buf := &bytes.Buffer{} + + g.It("opens a file if it exists on the system", func() { + err := rfs.CreateServerFile("test.txt", "testing") + g.Assert(err).IsNil() + + err = fs.Readfile("test.txt", buf) + g.Assert(err).IsNil() + g.Assert(buf.String()).Equal("testing") + }) + + g.It("returns an error if the file does not exist", func() { + err := fs.Readfile("test.txt", buf) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + }) + + g.It("returns an error if the \"file\" is a directory", func() { + err := os.Mkdir(filepath.Join(rfs.root, "/server/test.txt"), 0755) + g.Assert(err).IsNil() + + err = fs.Readfile("test.txt", buf) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrIsDirectory)).IsTrue() + }) + + g.It("cannot open a file outside the root directory", func() { + err := rfs.CreateServerFile("/../test.txt", "testing") + g.Assert(err).IsNil() + + err = fs.Readfile("/../test.txt", buf) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + + g.AfterEach(func() { + buf.Truncate(0) + fs.diskUsed = 0 + rfs.reset() + }) + }) +} + +func TestFilesystem_Writefile(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + g.Describe("Open and WriteFile", func() { + buf := &bytes.Buffer{} + + // Test that a file can be written to the disk and that the disk space used as a result + // is updated correctly in the end. + g.It("can create a new file", func() { + r := bytes.NewReader([]byte("test file content")) + + g.Assert(fs.diskUsed).Equal(int64(0)) + + err := fs.Writefile("test.txt", r) + g.Assert(err).IsNil() + + err = fs.Readfile("test.txt", buf) + g.Assert(err).IsNil() + g.Assert(buf.String()).Equal("test file content") + g.Assert(fs.diskUsed).Equal(r.Size()) + }) + + g.It("can create a new file inside a nested directory with leading slash", func() { + r := bytes.NewReader([]byte("test file content")) + + err := fs.Writefile("/some/nested/test.txt", r) + g.Assert(err).IsNil() + + err = fs.Readfile("/some/nested/test.txt", buf) + g.Assert(err).IsNil() + g.Assert(buf.String()).Equal("test file content") + }) + + g.It("can create a new file inside a nested directory without a trailing slash", func() { + r := bytes.NewReader([]byte("test file content")) + + err := fs.Writefile("some/../foo/bar/test.txt", r) + g.Assert(err).IsNil() + + err = fs.Readfile("foo/bar/test.txt", buf) + g.Assert(err).IsNil() + g.Assert(buf.String()).Equal("test file content") + }) + + g.It("cannot create a file outside the root directory", func() { + r := bytes.NewReader([]byte("test file content")) + + err := fs.Writefile("/some/../foo/../../test.txt", r) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + + g.It("cannot write a file that exceedes the disk limits", func() { + fs.diskLimit = 1024 + + b := make([]byte, 1025) + _, err := rand.Read(b) + g.Assert(err).IsNil() + g.Assert(len(b)).Equal(1025) + + r := bytes.NewReader(b) + err = fs.Writefile("test.txt", r) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrNotEnoughDiskSpace)).IsTrue() + }) + + g.It("updates the total space used when a file is appended to", func() { + fs.diskUsed = 100 + + b := make([]byte, 100) + _, _ = rand.Read(b) + + r := bytes.NewReader(b) + err := fs.Writefile("test.txt", r) + g.Assert(err).IsNil() + g.Assert(fs.diskUsed).Equal(int64(200)) + + // If we write less data than already exists, we should expect the total + // disk used to be decremented. + b = make([]byte, 50) + _, _ = rand.Read(b) + + r = bytes.NewReader(b) + err = fs.Writefile("test.txt", r) + g.Assert(err).IsNil() + g.Assert(fs.diskUsed).Equal(int64(150)) + }) + + g.It("truncates the file when writing new contents", func() { + r := bytes.NewReader([]byte("original data")) + err := fs.Writefile("test.txt", r) + g.Assert(err).IsNil() + + r = bytes.NewReader([]byte("new data")) + err = fs.Writefile("test.txt", r) + g.Assert(err).IsNil() + + err = fs.Readfile("test.txt", buf) + g.Assert(err).IsNil() + g.Assert(buf.String()).Equal("new data") + }) + + g.AfterEach(func() { + buf.Truncate(0) + rfs.reset() + fs.diskUsed = 0 + fs.diskLimit = 0 + }) + }) +} + +func TestFilesystem_CreateDirectory(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + g.Describe("CreateDirectory", func() { + g.It("should create missing directories automatically", func() { + err := fs.CreateDirectory("test", "foo/bar/baz") + g.Assert(err).IsNil() + + st, err := rfs.StatServerFile("foo/bar/baz/test") + g.Assert(err).IsNil() + g.Assert(st.IsDir()).IsTrue() + g.Assert(st.Name()).Equal("test") + }) + + g.It("should work with leading and trailing slashes", func() { + err := fs.CreateDirectory("test", "/foozie/barzie/bazzy/") + g.Assert(err).IsNil() + + st, err := rfs.StatServerFile("foozie/barzie/bazzy/test") + g.Assert(err).IsNil() + g.Assert(st.IsDir()).IsTrue() + g.Assert(st.Name()).Equal("test") + }) + + 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.It("should not increment the disk usage", func() { + err := fs.CreateDirectory("test", "/") + g.Assert(err).IsNil() + g.Assert(fs.diskUsed).Equal(int64(0)) + }) + + g.AfterEach(func() { + rfs.reset() + }) + }) +} + +func TestFilesystem_Rename(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + g.Describe("Rename", func() { + g.BeforeEach(func() { + if err := rfs.CreateServerFile("source.txt", "text content"); err != nil { + panic(err) + } + }) + + g.It("returns an error if the target already exists", func() { + err := rfs.CreateServerFile("target.txt", "taget content") + g.Assert(err).IsNil() + + err = fs.Rename("source.txt", "target.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrExist)).IsTrue() + }) + + g.It("returns an error if the final destination is the root directory", func() { + err := fs.Rename("source.txt", "/") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrExist)).IsTrue() + }) + + g.It("returns an error if the source destination is the root directory", func() { + err := fs.Rename("source.txt", "/") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrExist)).IsTrue() + }) + + 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.It("does not allow renaming from a location outside the root", func() { + err := rfs.CreateServerFile("/../ext-source.txt", "taget content") + + err = fs.Rename("/../ext-source.txt", "target.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + + g.It("allows a file to be renamed", func() { + err := fs.Rename("source.txt", "target.txt") + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("source.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + + st, err := rfs.StatServerFile("target.txt") + g.Assert(err).IsNil() + g.Assert(st.Name()).Equal("target.txt") + g.Assert(st.Size()).IsNotZero() + }) + + g.It("allows a folder to be renamed", func() { + err := os.Mkdir(filepath.Join(rfs.root, "/server/source_dir"), 0755) + g.Assert(err).IsNil() + + err = fs.Rename("source_dir", "target_dir") + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("source_dir") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + + st, err := rfs.StatServerFile("target_dir") + g.Assert(err).IsNil() + g.Assert(st.IsDir()).IsTrue() + }) + + g.It("returns an error if the source does not exist", func() { + err := fs.Rename("missing.txt", "target.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + }) + + g.It("creates directories if they are missing", func() { + err := fs.Rename("source.txt", "nested/folder/target.txt") + g.Assert(err).IsNil() + + st, err := rfs.StatServerFile("nested/folder/target.txt") + g.Assert(err).IsNil() + g.Assert(st.Name()).Equal("target.txt") + }) + + g.AfterEach(func() { + rfs.reset() + }) + }) +} + +func TestFilesystem_Copy(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + g.Describe("Copy", func() { + g.BeforeEach(func() { + if err := rfs.CreateServerFile("source.txt", "text content"); err != nil { + panic(err) + } + + fs.diskUsed = int64(utf8.RuneCountInString("test content")) + }) + + g.It("should return an error if the source does not exist", func() { + err := fs.Copy("foo.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + }) + + g.It("should return an error if the source is outside the root", func() { + err := rfs.CreateServerFile("/../ext-source.txt", "text content") + + err = fs.Copy("../ext-source.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + + g.It("should return an error if the source directory is outside the root", func() { + err := os.MkdirAll(filepath.Join(rfs.root, "/nested/in/dir"), 0755) + g.Assert(err).IsNil() + + err = rfs.CreateServerFile("/../nested/in/dir/ext-source.txt", "external content") + g.Assert(err).IsNil() + + err = fs.Copy("../nested/in/dir/ext-source.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + + err = fs.Copy("nested/in/../../../nested/in/dir/ext-source.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + + g.It("should return an error if the source is a directory", func() { + err := os.Mkdir(filepath.Join(rfs.root, "/server/dir"), 0755) + g.Assert(err).IsNil() + + err = fs.Copy("dir") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + }) + + g.It("should return an error if there is not space to copy the file", func() { + fs.diskLimit = 2 + + err := fs.Copy("source.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrNotEnoughDiskSpace)).IsTrue() + }) + + g.It("should create a copy of the file and increment the disk used", func() { + err := fs.Copy("source.txt") + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("source.txt") + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("source copy.txt") + g.Assert(err).IsNil() + }) + + g.It("should create a copy of the file with a suffix if a copy already exists", func() { + err := fs.Copy("source.txt") + g.Assert(err).IsNil() + + err = fs.Copy("source.txt") + g.Assert(err).IsNil() + + r := []string{"source.txt", "source copy.txt", "source copy 1.txt"} + + for _, name := range r { + _, err = rfs.StatServerFile(name) + g.Assert(err).IsNil() + } + + g.Assert(fs.diskUsed).Equal(int64(utf8.RuneCountInString("test content")) * 3) + }) + + g.It("should create a copy inside of a directory", func() { + err := os.MkdirAll(filepath.Join(rfs.root, "/server/nested/in/dir"), 0755) + g.Assert(err).IsNil() + + err = rfs.CreateServerFile("nested/in/dir/source.txt", "test content") + g.Assert(err).IsNil() + + err = fs.Copy("nested/in/dir/source.txt") + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("nested/in/dir/source.txt") + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("nested/in/dir/source copy.txt") + g.Assert(err).IsNil() + }) + + g.AfterEach(func() { + rfs.reset() + fs.diskUsed = 0 + fs.diskLimit = 0 + }) + }) +} + +func TestFilesystem_Delete(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + + g.Describe("Delete", func() { + g.BeforeEach(func() { + if err := rfs.CreateServerFile("source.txt", "test content"); err != nil { + panic(err) + } + + fs.diskUsed = int64(utf8.RuneCountInString("test content")) + }) + + g.It("does not delete files outside the root directory", func() { + err := rfs.CreateServerFile("/../ext-source.txt", "external content") + + err = fs.Delete("../ext-source.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() + }) + + g.It("does not allow the deletion of the root directory", func() { + err := fs.Delete("/") + g.Assert(err).IsNotNil() + g.Assert(err.Error()).Equal("cannot delete root server directory") + }) + + g.It("does not return an error if the target does not exist", func() { + err := fs.Delete("missing.txt") + g.Assert(err).IsNil() + + st, err := rfs.StatServerFile("source.txt") + g.Assert(err).IsNil() + g.Assert(st.Name()).Equal("source.txt") + }) + + g.It("deletes files and subtracts their size from the disk usage", func() { + err := fs.Delete("source.txt") + g.Assert(err).IsNil() + + _, err = rfs.StatServerFile("source.txt") + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + + g.Assert(fs.diskUsed).Equal(int64(0)) + }) + + g.It("deletes all items inside a directory if the directory is deleted", func() { + sources := []string{ + "foo/source.txt", + "foo/bar/source.txt", + "foo/bar/baz/source.txt", + } + + err := os.MkdirAll(filepath.Join(rfs.root, "/server/foo/bar/baz"), 0755) + g.Assert(err).IsNil() + + for _, s := range sources { + err = rfs.CreateServerFile(s, "test content") + g.Assert(err).IsNil() + } + + fs.diskUsed = int64(utf8.RuneCountInString("test content") * 3) + + err = fs.Delete("foo") + g.Assert(err).IsNil() + g.Assert(fs.diskUsed).Equal(int64(0)) + + for _, s := range sources { + _, err = rfs.StatServerFile(s) + g.Assert(err).IsNotNil() + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() + } + }) + + g.AfterEach(func() { + rfs.reset() + fs.diskUsed = 0 + fs.diskLimit = 0 + }) + }) +} diff --git a/server/filesystem/path.go b/server/filesystem/path.go index dd98022..6476a9d 100644 --- a/server/filesystem/path.go +++ b/server/filesystem/path.go @@ -92,27 +92,6 @@ func (fs *Filesystem) unsafeIsInDataDirectory(p string) bool { return strings.HasPrefix(strings.TrimSuffix(p, "/")+"/", strings.TrimSuffix(fs.Path(), "/")+"/") } -// Helper function to keep some of the codebase a little cleaner. Returns a "safe" version of the path -// joined with a file. This is important because you cannot just assume that appending a file to a cleaned -// path will result in a cleaned path to that file. For example, imagine you have the following scenario: -// -// my_bad_file -> symlink:/etc/passwd -// -// cleaned := SafePath("../../etc") -> "/" -// filepath.Join(cleaned, my_bad_file) -> "/my_bad_file" -// -// You might think that "/my_bad_file" is fine since it isn't pointing to the original "../../etc/my_bad_file". -// However, this doesn't account for symlinks where the file might be pointing outside of the directory, so -// calling a function such as Chown against it would chown the symlinked location, and not the file within the -// Wings daemon. -func (fs *Filesystem) SafeJoin(dir string, f os.FileInfo) (string, error) { - if f.Mode()&os.ModeSymlink != 0 { - return fs.SafePath(filepath.Join(dir, f.Name())) - } - - return filepath.Join(dir, f.Name()), nil -} - // Executes the fs.SafePath function in parallel against an array of paths. If any of the calls // fails an error will be returned. func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) {