Skip to content

Commit 847d08b

Browse files
hacdiasclaude
andcommitted
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>
1 parent 0231b7e commit 847d08b

4 files changed

Lines changed: 77 additions & 1 deletion

File tree

files/file.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,17 @@ func stat(opts *FileOptions) (*FileInfo, error) {
133133
return file, nil
134134
}
135135

136+
// The path is a symlink. Refuse to follow it if its on-disk target escapes
137+
// the user's scoped root; otherwise a symlink that lives lexically inside
138+
// the scope but points outside it would let a restricted user read, write,
139+
// or share files beyond their boundary.
140+
if file != nil && file.IsSymlink {
141+
ok, scopeErr := WithinScope(opts.Fs, opts.Path)
142+
if scopeErr != nil || !ok {
143+
return nil, os.ErrPermission
144+
}
145+
}
146+
136147
// fs doesn't support afero.Lstater interface or the file is a symlink
137148
info, err := opts.Fs.Stat(opts.Path)
138149
if err != nil {
@@ -165,6 +176,50 @@ func stat(opts *FileOptions) (*FileInfo, error) {
165176
return file, nil
166177
}
167178

179+
// WithinScope reports whether the on-disk target of p — after resolving any
180+
// symbolic links — stays within the scoped root of fsys. It exists to stop a
181+
// symlink that lives lexically inside a user's scope but points outside it
182+
// from being followed for reads, writes, or shares.
183+
//
184+
// Paths that do not exist yet (e.g. a brand-new file being created) are
185+
// validated against their nearest existing ancestor, so legitimate new files
186+
// are always allowed. For a filesystem that is not scoped with BasePathFs the
187+
// check is a no-op and returns true.
188+
//
189+
// Note: a dangling symlink whose target does not yet exist resolves to its
190+
// containing directory and is therefore allowed; writing through such a link
191+
// could still create a file outside the scope. Callers that create files
192+
// should treat this as best-effort and rely on rejecting existing escaping
193+
// symlinks, which covers the disclosure and overwrite vectors.
194+
func WithinScope(fsys afero.Fs, p string) (bool, error) {
195+
bfs, ok := fsys.(*afero.BasePathFs)
196+
if !ok {
197+
// Not a scoped filesystem; nothing to enforce.
198+
return true, nil
199+
}
200+
201+
root, err := filepath.EvalSymlinks(afero.FullBaseFsPath(bfs, "/"))
202+
if err != nil {
203+
return false, err
204+
}
205+
206+
target := afero.FullBaseFsPath(bfs, p)
207+
resolved, err := filepath.EvalSymlinks(target)
208+
for errors.Is(err, fs.ErrNotExist) {
209+
parent := filepath.Dir(target)
210+
if parent == target {
211+
break
212+
}
213+
target = parent
214+
resolved, err = filepath.EvalSymlinks(target)
215+
}
216+
if err != nil {
217+
return false, err
218+
}
219+
220+
return resolved == root || strings.HasPrefix(resolved, root+string(filepath.Separator)), nil
221+
}
222+
168223
// Checksum checksums a given File for a given User, using a specific
169224
// algorithm. The checksums data is saved on File object.
170225
func (i *FileInfo) Checksum(algo string) error {

http/auth.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020

2121
const (
2222
DefaultTokenExpirationTime = time.Hour * 2
23+
24+
maxAuthBodySize = 1 << 20 // 1 MiB
2325
)
2426

2527
type userInfo struct {
@@ -120,6 +122,10 @@ func withAdmin(fn handleFunc) handleFunc {
120122

121123
func loginHandler(tokenExpireTime time.Duration) handleFunc {
122124
return func(w http.ResponseWriter, r *http.Request, d *data) (int, error) {
125+
if r.Body != nil {
126+
r.Body = http.MaxBytesReader(w, r.Body, maxAuthBodySize)
127+
}
128+
123129
auther, err := d.store.Auth.Get(d.settings.AuthMethod)
124130
if err != nil {
125131
return http.StatusInternalServerError, err
@@ -142,7 +148,7 @@ type signupBody struct {
142148
Password string `json:"password"`
143149
}
144150

145-
var signupHandler = func(_ http.ResponseWriter, r *http.Request, d *data) (int, error) {
151+
var signupHandler = func(w http.ResponseWriter, r *http.Request, d *data) (int, error) {
146152
if !d.settings.Signup {
147153
return http.StatusMethodNotAllowed, nil
148154
}
@@ -151,6 +157,8 @@ var signupHandler = func(_ http.ResponseWriter, r *http.Request, d *data) (int,
151157
return http.StatusBadRequest, nil
152158
}
153159

160+
r.Body = http.MaxBytesReader(w, r.Body, maxAuthBodySize)
161+
154162
info := &signupBody{}
155163
err := json.NewDecoder(r.Body).Decode(info)
156164
if err != nil {

http/raw.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,12 @@ func getFiles(d *data, path, commonPath string) ([]archives.FileInfo, error) {
125125
nameInArchive := strings.TrimPrefix(path, commonPath)
126126
nameInArchive = strings.TrimPrefix(nameInArchive, string(filepath.Separator))
127127
nameInArchive = filepath.ToSlash(nameInArchive)
128+
// filepath.ToSlash only rewrites the host separator, so on a Linux
129+
// host a stored backslash survives and is emitted verbatim into the
130+
// archive. Windows extractors then treat "\" as a path separator,
131+
// allowing the entry to escape the extraction directory (zip-slip).
132+
// Strip Windows separators regardless of host OS.
133+
nameInArchive = strings.ReplaceAll(nameInArchive, "\\", "/")
128134

129135
archiveFiles = append(archiveFiles, archives.FileInfo{
130136
FileInfo: info,

http/resource.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,13 @@ func addVersionSuffix(source string, afs afero.Fs) string {
295295
}
296296

297297
func writeFile(afs afero.Fs, dst string, in io.Reader, fileMode, dirMode fs.FileMode) (os.FileInfo, error) {
298+
// Refuse to write through a symlink that escapes the user's scope, so an
299+
// overwrite of an existing escaping symlink cannot modify a file outside
300+
// the boundary.
301+
if ok, err := files.WithinScope(afs, dst); err != nil || !ok {
302+
return nil, os.ErrPermission
303+
}
304+
298305
dir, _ := path.Split(dst)
299306
err := afs.MkdirAll(dir, dirMode)
300307
if err != nil {

0 commit comments

Comments
 (0)