diff --git a/files/file.go b/files/file.go index 2ba432dc..3b38f314 100644 --- a/files/file.go +++ b/files/file.go @@ -133,6 +133,17 @@ func stat(opts *FileOptions) (*FileInfo, error) { return file, nil } + // The path is a symlink. Refuse to follow it if its on-disk target escapes + // the user's scoped root; otherwise a symlink that lives lexically inside + // the scope but points outside it would let a restricted user read, write, + // or share files beyond their boundary. + if file != nil && file.IsSymlink { + ok, scopeErr := WithinScope(opts.Fs, opts.Path) + if scopeErr != nil || !ok { + return nil, os.ErrPermission + } + } + // fs doesn't support afero.Lstater interface or the file is a symlink info, err := opts.Fs.Stat(opts.Path) if err != nil { @@ -165,6 +176,50 @@ func stat(opts *FileOptions) (*FileInfo, error) { return file, nil } +// WithinScope reports whether the on-disk target of p — after resolving any +// symbolic links — stays within the scoped root of fsys. It exists to stop a +// symlink that lives lexically inside a user's scope but points outside it +// from being followed for reads, writes, or shares. +// +// Paths that do not exist yet (e.g. a brand-new file being created) are +// validated against their nearest existing ancestor, so legitimate new files +// are always allowed. For a filesystem that is not scoped with BasePathFs the +// check is a no-op and returns true. +// +// Note: a dangling symlink whose target does not yet exist resolves to its +// containing directory and is therefore allowed; writing through such a link +// could still create a file outside the scope. Callers that create files +// should treat this as best-effort and rely on rejecting existing escaping +// symlinks, which covers the disclosure and overwrite vectors. +func WithinScope(fsys afero.Fs, p string) (bool, error) { + bfs, ok := fsys.(*afero.BasePathFs) + if !ok { + // Not a scoped filesystem; nothing to enforce. + return true, nil + } + + root, err := filepath.EvalSymlinks(afero.FullBaseFsPath(bfs, "/")) + if err != nil { + return false, err + } + + target := afero.FullBaseFsPath(bfs, p) + resolved, err := filepath.EvalSymlinks(target) + for errors.Is(err, fs.ErrNotExist) { + parent := filepath.Dir(target) + if parent == target { + break + } + target = parent + resolved, err = filepath.EvalSymlinks(target) + } + if err != nil { + return false, err + } + + return resolved == root || strings.HasPrefix(resolved, root+string(filepath.Separator)), nil +} + // Checksum checksums a given File for a given User, using a specific // algorithm. The checksums data is saved on File object. func (i *FileInfo) Checksum(algo string) error { diff --git a/http/auth.go b/http/auth.go index 59f15a36..4381e86c 100644 --- a/http/auth.go +++ b/http/auth.go @@ -20,6 +20,8 @@ import ( const ( DefaultTokenExpirationTime = time.Hour * 2 + + maxAuthBodySize = 1 << 20 // 1 MiB ) type userInfo struct { @@ -120,6 +122,10 @@ func withAdmin(fn handleFunc) handleFunc { func loginHandler(tokenExpireTime time.Duration) handleFunc { return func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { + if r.Body != nil { + r.Body = http.MaxBytesReader(w, r.Body, maxAuthBodySize) + } + auther, err := d.store.Auth.Get(d.settings.AuthMethod) if err != nil { return http.StatusInternalServerError, err @@ -142,7 +148,7 @@ type signupBody struct { Password string `json:"password"` } -var signupHandler = func(_ http.ResponseWriter, r *http.Request, d *data) (int, error) { +var signupHandler = func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { if !d.settings.Signup { return http.StatusMethodNotAllowed, nil } @@ -151,6 +157,8 @@ var signupHandler = func(_ http.ResponseWriter, r *http.Request, d *data) (int, return http.StatusBadRequest, nil } + r.Body = http.MaxBytesReader(w, r.Body, maxAuthBodySize) + info := &signupBody{} err := json.NewDecoder(r.Body).Decode(info) if err != nil { diff --git a/http/raw.go b/http/raw.go index b5c86643..35e474ae 100644 --- a/http/raw.go +++ b/http/raw.go @@ -125,6 +125,12 @@ func getFiles(d *data, path, commonPath string) ([]archives.FileInfo, error) { nameInArchive := strings.TrimPrefix(path, commonPath) nameInArchive = strings.TrimPrefix(nameInArchive, string(filepath.Separator)) nameInArchive = filepath.ToSlash(nameInArchive) + // filepath.ToSlash only rewrites the host separator, so on a Linux + // host a stored backslash survives and is emitted verbatim into the + // archive. Windows extractors then treat "\" as a path separator, + // allowing the entry to escape the extraction directory (zip-slip). + // Strip Windows separators regardless of host OS. + nameInArchive = strings.ReplaceAll(nameInArchive, "\\", "/") archiveFiles = append(archiveFiles, archives.FileInfo{ FileInfo: info, diff --git a/http/resource.go b/http/resource.go index e5c519ea..9f7bfb01 100644 --- a/http/resource.go +++ b/http/resource.go @@ -295,6 +295,13 @@ func addVersionSuffix(source string, afs afero.Fs) string { } func writeFile(afs afero.Fs, dst string, in io.Reader, fileMode, dirMode fs.FileMode) (os.FileInfo, error) { + // Refuse to write through a symlink that escapes the user's scope, so an + // overwrite of an existing escaping symlink cannot modify a file outside + // the boundary. + if ok, err := files.WithinScope(afs, dst); err != nil || !ok { + return nil, os.ErrPermission + } + dir, _ := path.Split(dst) err := afs.MkdirAll(dir, dirMode) if err != nil {