mirror of
https://github.com/filebrowser/filebrowser.git
synced 2026-07-18 00:45:47 +00:00
fix: address three security disclosures (archive traversal, login DoS, symlink escape)
- http/raw.go: strip Windows backslash separators from archive entry names on any host. filepath.ToSlash is a no-op for "\" on Linux, so a stored backslash filename was emitted verbatim and could escape the extraction directory on Windows extractors (zip-slip). (GHSA-gxjx-7m74-hcq8) - http/auth.go: cap the login and signup request bodies with http.MaxBytesReader (1 MiB). The JSON decoder previously read an arbitrarily large password into memory before bcrypt truncated it, enabling unauthenticated memory-exhaustion DoS. (GHSA-w5fm-68j4-fpc4) - files/file.go, http/resource.go: add files.WithinScope and refuse to follow a symlink whose on-disk target escapes the user's scoped root, on both the read path (stat) and the write path (writeFile). Prevents a scoped user from reading/overwriting/sharing files outside their scope via a pre-existing escaping symlink. (GHSA-239w-m3h6-ch8v) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
0231b7ebdf
commit
847d08bdd1
4 changed files with 77 additions and 1 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
10
http/auth.go
10
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 {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue