From a635cdd6b288e5fd172947f7161a92356bd13eb2 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 15 Jul 2020 21:16:08 -0700 Subject: [PATCH] Support unarching in a better fashion with zip-slip protections and size checking --- router/router_server_files.go | 10 ++- server/filesystem.go | 62 ++++++-------- server/filesystem_unarchive.go | 151 +++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 40 deletions(-) create mode 100644 server/filesystem_unarchive.go diff --git a/router/router_server_files.go b/router/router_server_files.go index 5072ef2..a7db31e 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -290,9 +290,15 @@ func postServerDecompressFiles(c *gin.Context) { return } - if !s.Filesystem.HasSpaceAvailable() { + hasSpace, err := s.Filesystem.SpaceAvailableForDecompression(data.RootPath, data.File) + if err != nil { + TrackedServerError(err, s).AbortWithServerError(c) + return + } + + if !hasSpace { c.AbortWithStatusJSON(http.StatusConflict, gin.H{ - "error": "This server does not have enough available disk space to decompress an archive.", + "error": "This server does not have enough available disk space to decompress this archive.", }) return } diff --git a/server/filesystem.go b/server/filesystem.go index d20adb0..5ea2f16 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "github.com/gabriel-vasile/mimetype" - "github.com/mholt/archiver/v3" "github.com/pkg/errors" "github.com/pterodactyl/wings/config" "github.com/pterodactyl/wings/server/backup" @@ -165,34 +164,11 @@ func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) { func (fs *Filesystem) HasSpaceAvailable() bool { space := fs.Server.Build.DiskSpace - // If we have a match in the cache, use that value in the return. No need to perform an expensive - // disk operation, even if this is an empty value. - if x, exists := fs.Server.Cache.Get("disk_used"); exists { - fs.Server.Resources.Disk = x.(int64) - - // This check is here to ensure that true is always returned if the server has unlimited disk space. - // See the end of this method for more information (the other `if space <= 0`). - if space <= 0 { - return true - } - - return (x.(int64) / 1000.0 / 1000.0) <= space - } - - // If there is no size its either because there is no data (in which case running this function - // will have effectively no impact), or there is nothing in the cache, in which case we need to - // grab the size of their data directory. This is a taxing operation, so we want to store it in - // the cache once we've gotten it. - size, err := fs.DirectorySize("/") + size, err := fs.getCachedDiskUsage() if err != nil { fs.Server.Log().WithField("error", err).Warn("failed to determine root server directory size") } - // Always cache the size, even if there is an error. We want to always return that value - // so that we don't cause an endless loop of determining the disk size if there is a temporary - // error encountered. - fs.Server.Cache.Set("disk_used", size, time.Second*60) - // Determine if their folder size, in bytes, is smaller than the amount of space they've // been allocated. fs.Server.Resources.Disk = size @@ -209,6 +185,29 @@ func (fs *Filesystem) HasSpaceAvailable() bool { return (size / 1000.0 / 1000.0) <= space } +// Internal helper function to allow other parts of the codebase to check the total used disk space +// as needed without overly taxing the system. This will prioritize the value from the cache to avoid +// excessive IO usage. We will only walk the filesystem and determine the size of the directory if there +// is no longer a cached value. +func (fs *Filesystem) getCachedDiskUsage() (int64, error) { + if x, exists := fs.Server.Cache.Get("disk_used"); exists { + return x.(int64), nil + } + + // If there is no size its either because there is no data (in which case running this function + // will have effectively no impact), or there is nothing in the cache, in which case we need to + // grab the size of their data directory. This is a taxing operation, so we want to store it in + // the cache once we've gotten it. + size, err := fs.DirectorySize("/") + + // Always cache the size, even if there is an error. We want to always return that value + // so that we don't cause an endless loop of determining the disk size if there is a temporary + // error encountered. + fs.Server.Cache.Set("disk_used", size, time.Second*60) + + return size, err +} + // Determines the directory size of a given location by running parallel tasks to iterate // through all of the folders. Returns the size in bytes. This can be a fairly taxing operation // on locations with tons of files, so it is recommended that you cache the output. @@ -743,16 +742,3 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er return a.Create(d, context.Background()) } - -// Decompress a file in a given directory by using the archiver tool to infer the file -// type and go from there. -func (fs *Filesystem) DecompressFile(dir string, file string) error { - source, err := fs.SafePath(filepath.Join(dir, file)) - if err != nil { - return errors.WithStack(err) - } - - dest := strings.TrimSuffix(source, filepath.Base(source)) - - return archiver.Unarchive(source, dest) -} diff --git a/server/filesystem_unarchive.go b/server/filesystem_unarchive.go new file mode 100644 index 0000000..dc07a82 --- /dev/null +++ b/server/filesystem_unarchive.go @@ -0,0 +1,151 @@ +package server + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "context" + "fmt" + "github.com/mholt/archiver/v3" + "github.com/pkg/errors" + "golang.org/x/sync/errgroup" + "io" + "os" + "path/filepath" + "reflect" + "strings" + "sync" + "sync/atomic" +) + +// Look through a given archive and determine if decompressing it would put the server over +// its allocated disk space limit. +func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (bool, error) { + // Don't waste time trying to determine this if we know the server will have the space for + // it since there is no limit. + if fs.Server.Build.DiskSpace <= 0 { + return true, nil + } + + source, err := fs.SafePath(filepath.Join(dir, file)) + if err != nil { + return false, err + } + + wg := new(sync.WaitGroup) + + var dirSize int64 + var cErr error + // Get the cached size in a parallel process so that if it is not cached we are not + // waiting an unnecessary amount of time on this call. + go func() { + wg.Add(1) + defer wg.Done() + + dirSize, cErr = fs.getCachedDiskUsage() + }() + + var size int64 + // In a seperate thread, walk over the archive and figure out just how large the final + // output would be from dearchiving it. + go func() { + wg.Add(1) + defer wg.Done() + + // Walk all of the files and calculate the total decompressed size of this archive. + archiver.Walk(source, func(f archiver.File) error { + atomic.AddInt64(&size, f.Size()) + + return nil + }) + }() + + wg.Wait() + + return ((dirSize + size) / 1000.0 / 1000.0) <= fs.Server.Build.DiskSpace, cErr +} + +// Decompress a file in a given directory by using the archiver tool to infer the file +// type and go from there. This will walk over all of the files within the given archive +// and ensure that there is not a zip-slip attack being attempted by validating that the +// final path is within the server data directory. +func (fs *Filesystem) DecompressFile(dir string, file string) error { + source, err := fs.SafePath(filepath.Join(dir, file)) + if err != nil { + return errors.WithStack(err) + } + + g, ctx := errgroup.WithContext(context.Background()) + + // Walk over all of the files spinning up an additional go-routine for each file we've encountered + // and then extract that file from the archive and write it to the disk. If any part of this process + // encounters an error the entire process will be stopped. + archiver.Walk(source, func(f archiver.File) error { + // Don't waste time with directories, we don't need to create them if they have no contents, and + // we will ensure the directory exists when opening the file for writing anyways. + if f.IsDir() { + return nil + } + + fi := f + g.Go(func() error { + select { + case <-ctx.Done(): + return ctx.Err() + default: + return fs.extractFileFromArchive(fi) + } + }) + + return nil + }) + + // Wait until everything is done, and then return. + if err := g.Wait(); err != nil { + return errors.WithStack(err) + } + + return nil +} + +// Extracts a single file from the archive and writes it to the disk after verifying that it will end +// up in the server data directory. +func (fs *Filesystem) extractFileFromArchive(f archiver.File) error { + var name string + + switch s := f.Sys().(type) { + case *tar.Header: + name = s.Name + case *gzip.Header: + name = s.Name + case *zip.FileHeader: + name = s.Name + default: + return errors.New(fmt.Sprintf("could not parse underlying data source with type %s", reflect.TypeOf(s).String())) + } + + // Guard against a zip-slip attack and prevent writing a file to a destination outside of + // the server root directory. + p, err := fs.SafePath(name) + if err != nil { + return err + } + + // Ensure the directory structure for this file exists before trying to write the file + // to the disk, otherwise we'll have some unexpected fun. + if err := os.MkdirAll(strings.TrimSuffix(p, filepath.Base(p)), 0755); err != nil { + return err + } + + // Open the file and truncate it if it already exists. + o, err := os.OpenFile(p, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return err + } + + defer o.Close() + + _, cerr := io.Copy(o, f) + + return cerr +}