openclaw/CHFS-NG
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

security: remediate review B findings

Browse Source
This commit is contained in:
kory
2026-10-03 01:37:02 +08:00
parent ca4d54d5b3
commit 56b05b11d9
11 changed files with 674 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/review_b_audit_test.go
+106
View File
@@ -0,0 +1,106 @@
package api
import (
"image"
"image/color"
"image/png"
"net/http/httptest"
"os"
"path/filepath"
"testing"
"chfs-ng/internal/auth"
"chfs-ng/internal/storage"
)
func TestAuditExistEnforcesReadPermission(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "secret.txt"), []byte("x"), 0600); err != nil {
t.Fatal(err)
}
s := &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Guest: &auth.Account{Mask: auth.None}}}
rr := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/chfs/exist?file=secret.txt", nil)
s.exist(rr, req)
if rr.Code != 403 {
t.Fatalf("exist status=%d want 403", rr.Code)
}
}
func TestAuditThumbnailRejectsOversizeBeforeDecode(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "big.bin"), make([]byte, 64), 0600); err != nil {
t.Fatal(err)
}
s := &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Guest: &auth.Account{Mask: auth.Read}}, ImagePreview: true, ThumbnailMaxSize: 32}
rr := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/chfs/thumbnail?file=big.bin", nil)
s.thumbnail(rr, req)
if rr.Code != 413 {
t.Fatalf("thumbnail status=%d want 413", rr.Code)
}
}
func TestAuditZipLimitsUseOriginalFileSizes(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "a.txt"), make([]byte, 100), 0600); err != nil {
t.Fatal(err)
}
s := &Server{Store: storage.Local{Root: root}, ZipMaxSize: 99, ZipMaxFiles: 10}
a := &auth.Account{Mask: auth.Read}
if err := s.checkZipLimits(a, []string{""}); err == nil {
t.Fatal("expected zip size limit")
}
}
func TestAuditSearchLimit(t *testing.T) {
root := t.TempDir()
for _, name := range []string{"a.txt", "b.txt", "c.txt"} {
if err := os.WriteFile(filepath.Join(root, name), []byte(name), 0600); err != nil {
t.Fatal(err)
}
}
s := &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Guest: &auth.Account{Mask: auth.Read}}, SearchMaxResults: 2}
rr := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/chfs/search?str=.txt", nil)
s.search(rr, req)
if rr.Code != 200 {
t.Fatalf("search status=%d", rr.Code)
}
if len(rr.Body.Bytes()) == 0 || !contains(rr.Body.String(), `"truncated":true`) {
t.Fatalf("search did not report truncation: %s", rr.Body.String())
}
}
func contains(s, sub string) bool {
return len(s) >= len(sub) && (s == sub || func() bool {
for i := 0; i+len(sub) <= len(s); i++ {
if s[i:i+len(sub)] == sub {
return true
}
}
return false
}())
}
func TestAuditThumbnailPixelLimit(t *testing.T) {
root := t.TempDir()
img := image.NewRGBA(image.Rect(0, 0, 20, 20))
img.Set(0, 0, color.White)
f, err := os.Create(filepath.Join(root, "large.png"))
if err != nil {
t.Fatal(err)
}
if err := png.Encode(f, img); err != nil {
f.Close()
t.Fatal(err)
}
f.Close()
s := &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Guest: &auth.Account{Mask: auth.Read}}, ImagePreview: true, ThumbnailMaxPixels: 100}
rr := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/chfs/thumbnail?file=large.png", nil)
s.thumbnail(rr, req)
if rr.Code != 415 {
t.Fatalf("thumbnail pixel-limit status=%d want 415", rr.Code)
}
}
internal/api/server.go
+176 -11
View File
@@ -47,6 +47,7 @@ type Server struct {
WebDAVRoot string
WebDAVFS webdav.FileSystem
Allow string
TrustedProxies string
MultiRoot bool
RemoveMode int
ImagePreview bool
@@ -56,6 +57,14 @@ type Server struct {
Log *opslog.Logger
UserStore *auth.UserStore
UserHome string
ThumbnailMaxSize int64
ThumbnailMaxPixels int64
ZipMaxSize int64
ZipMaxFiles int
UploadMaxSize int64
UploadMaxTotalSize int64
UploadMaxFiles int
SearchMaxResults int
ThumbnailCache *thumbnailCache
thumbnailCacheOnce sync.Once
}
@@ -138,11 +147,11 @@ func (s *Server) logOp(r *http.Request, action string) {
if a != nil {
user = a.Username
}
_ = s.Log.Write(clientIPString(r), user, action)
_ = s.Log.Write(s.clientIPString(r), user, action)
}
func clientIPString(r *http.Request) string {
ip := clientIP(r)
func (s *Server) clientIPString(r *http.Request) string {
ip := s.clientIP(r)
if ip == nil {
return "-"
}
@@ -516,6 +525,10 @@ func (s *Server) exist(w http.ResponseWriter, r *http.Request) {
return
}
p := strings.Trim(strings.ReplaceAll(raw, "\\", "/"), "/")
if !account(r, s).MaskFor(p).HasRead() {
writeJSONError(w, http.StatusForbidden, "has no privilege")
return
}
_, e := s.Store.Stat(p)
if e != nil {
w.WriteHeader(http.StatusNotFound)
@@ -639,7 +652,7 @@ func (s *Server) password(w http.ResponseWriter, r *http.Request) {
writeJSONError(w, http.StatusBadRequest, "invalid json")
return
}
if a.Password != req.CurrentPassword && a.PasswordHash != auth.PasswordHash(req.CurrentPassword) {
if a.Password != req.CurrentPassword && !auth.VerifyPasswordHash(a.PasswordHash, req.CurrentPassword) {
writeJSONError(w, http.StatusBadRequest, "current password incorrect")
return
}
@@ -855,8 +868,24 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) {
// This prevents duplicate/bad entries from leaving a partial upload.
seen := make(map[string]struct{})
var saved []string
var totalUpload int64
var uploadFiles int
for _, hs := range r.MultipartForm.File {
for _, h := range hs {
uploadFiles++
if s.UploadMaxFiles > 0 && uploadFiles > s.UploadMaxFiles {
http.Error(w, "upload file count limit exceeded", http.StatusRequestEntityTooLarge)
return
}
if s.UploadMaxSize > 0 && h.Size > s.UploadMaxSize {
http.Error(w, "upload file too large", http.StatusRequestEntityTooLarge)
return
}
totalUpload += h.Size
if s.UploadMaxTotalSize > 0 && totalUpload > s.UploadMaxTotalSize {
http.Error(w, "upload total size limit exceeded", http.StatusRequestEntityTooLarge)
return
}
name := filepath.ToSlash(h.Filename)
if name == "" || strings.HasPrefix(name, "/") || unsafePath(name) {
http.Error(w, "invalid filename", 400)
@@ -881,7 +910,7 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) {
for _, hs := range r.MultipartForm.File {
for _, h := range hs {
target := path.Join(dir, filepath.ToSlash(h.Filename))
if e := savePart(s.Store, target, h); e != nil {
if e := savePart(s.Store, target, h, s.UploadMaxSize); e != nil {
for _, previous := range saved {
_ = s.Store.Remove(previous)
}
@@ -908,7 +937,7 @@ func createExclusive(st storage.Storage, target string) (*os.File, error) {
return st.Create(target)
}
func savePart(st storage.Storage, target string, h *multipart.FileHeader) error {
func savePart(st storage.Storage, target string, h *multipart.FileHeader, maxSize int64) error {
in, e := h.Open()
if e != nil {
return e
@@ -919,8 +948,18 @@ func savePart(st storage.Storage, target string, h *multipart.FileHeader) error
return e
}
copyErr := func() error {
_, err := io.Copy(f, in)
return err
var src io.Reader = in
if maxSize > 0 {
src = io.LimitReader(in, maxSize+1)
}
n, err := io.Copy(f, src)
if err != nil {
return err
}
if maxSize > 0 && n > maxSize {
return fmt.Errorf("upload file too large")
}
return nil
}()
closeErr := f.Close()
if copyErr != nil {
@@ -953,8 +992,24 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) {
// Preflight the complete multipart batch before creating anything.
// This prevents duplicate/bad entries from leaving a partial upload.
seen := make(map[string]struct{})
var totalUpload int64
var uploadFiles int
for _, hs := range r.MultipartForm.File {
for _, h := range hs {
uploadFiles++
if s.UploadMaxFiles > 0 && uploadFiles > s.UploadMaxFiles {
http.Error(w, "upload file count limit exceeded", http.StatusRequestEntityTooLarge)
return
}
if s.UploadMaxSize > 0 && h.Size > s.UploadMaxSize {
http.Error(w, "upload file too large", http.StatusRequestEntityTooLarge)
return
}
totalUpload += h.Size
if s.UploadMaxTotalSize > 0 && totalUpload > s.UploadMaxTotalSize {
http.Error(w, "upload total size limit exceeded", http.StatusRequestEntityTooLarge)
return
}
name := filepath.ToSlash(h.Filename)
name = strings.TrimPrefix(name, "/")
if name == "" || unsafePath(name) {
@@ -981,7 +1036,7 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) {
for _, h := range hs {
name := strings.TrimPrefix(filepath.ToSlash(h.Filename), "/")
target := path.Join(dir, name)
if err := savePart(s.Store, target, h); err != nil {
if err := savePart(s.Store, target, h, s.UploadMaxSize); err != nil {
for _, previous := range saved {
_ = s.Store.Remove(previous)
}
@@ -1015,6 +1070,11 @@ func (s *Server) search(w http.ResponseWriter, r *http.Request) {
}
needle := strings.ToLower(strings.TrimSpace(r.URL.Query().Get("str")))
out := make([]searchFileInfo, 0)
maxResults := s.SearchMaxResults
if maxResults < 0 {
maxResults = 0
}
truncated := false
var walk func(string) error
walk = func(dir string) error {
items, err := s.Store.List(dir)
@@ -1022,6 +1082,10 @@ func (s *Server) search(w http.ResponseWriter, r *http.Request) {
return err
}
for _, fi := range items {
if maxResults > 0 && len(out) >= maxResults {
truncated = true
return nil
}
p := path.Join(dir, fi.Name())
if !a.MaskFor(p).HasRead() {
continue
@@ -1041,7 +1105,7 @@ func (s *Server) search(w http.ResponseWriter, r *http.Request) {
http.Error(w, err.Error(), 500)
return
}
writeJSON(w, map[string]any{"virtualRoot": 0, "currentMask": permString(rootMask), "files": out})
writeJSON(w, map[string]any{"virtualRoot": 0, "currentMask": permString(rootMask), "files": out, "truncated": truncated})
}
func sharedPath(parts ...string) string {
@@ -1183,6 +1247,10 @@ func (s *Server) thumbnail(w http.ResponseWriter, r *http.Request) {
http.NotFound(w, r)
return
}
if s.ThumbnailMaxSize > 0 && fi.Size() > s.ThumbnailMaxSize {
writeJSONError(w, http.StatusRequestEntityTooLarge, "thumbnail file too large")
return
}
key := thumbnailCacheKey{Path: file, Size: fi.Size(), ModTime: fi.ModTime().UnixNano()}
s.thumbnailCacheOnce.Do(func() {
if s.ThumbnailCache == nil {
@@ -1194,6 +1262,12 @@ func (s *Server) thumbnail(w http.ResponseWriter, r *http.Request) {
if err != nil {
return nil, err
}
if s.ThumbnailMaxPixels > 0 {
b := src.Bounds()
if int64(b.Dx())*int64(b.Dy()) > s.ThumbnailMaxPixels {
return nil, fmt.Errorf("thumbnail dimensions too large")
}
}
thumb := transform.Resize(src, 320, 240, transform.Linear)
var buf bytes.Buffer
if err := png.Encode(&buf, thumb); err != nil {
@@ -1287,6 +1361,62 @@ func (s *Server) qrcode(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write(data)
}
func (s *Server) zipStats(a *auth.Account, roots []string) (int64, int, error) {
var total int64
files := 0
seen := map[string]bool{}
var walk func(string) error
walk = func(p string) error {
if seen[p] {
return nil
}
seen[p] = true
fi, err := s.Store.Stat(p)
if err != nil {
return err
}
if !a.MaskFor(p).HasRead() {
return nil
}
if !fi.IsDir() {
files++
total += fi.Size()
return nil
}
items, err := s.Store.List(p)
if err != nil {
return err
}
for _, it := range items {
child := path.Join(p, it.Name())
if err := walk(child); err != nil {
return err
}
}
return nil
}
for _, root := range roots {
if err := walk(root); err != nil {
return 0, 0, err
}
}
return total, files, nil
}
func (s *Server) checkZipLimits(a *auth.Account, roots []string) error {
total, files, err := s.zipStats(a, roots)
if err != nil {
return err
}
if s.ZipMaxFiles > 0 && files > s.ZipMaxFiles {
return fmt.Errorf("zip file count limit exceeded")
}
if s.ZipMaxSize > 0 && total > s.ZipMaxSize {
return fmt.Errorf("zip size limit exceeded")
}
return nil
}
func (s *Server) downloadSelectedZip(w http.ResponseWriter, r *http.Request) {
a := account(r, s)
paths := r.URL.Query()["path"]
@@ -1312,6 +1442,10 @@ func (s *Server) downloadSelectedZip(w http.ResponseWriter, r *http.Request) {
return
}
}
if err := s.checkZipLimits(a, paths); err != nil {
writeJSONError(w, http.StatusRequestEntityTooLarge, err.Error())
return
}
w.Header().Set("Content-Type", "application/zip")
w.Header().Set("Content-Disposition", `attachment; filename="download.zip"`)
zw := zip.NewWriter(w)
@@ -1403,6 +1537,10 @@ func (s *Server) downloadDir(w http.ResponseWriter, r *http.Request) {
}
}
}
if err := s.checkZipLimits(a, []string{p}); err != nil {
writeJSONError(w, http.StatusRequestEntityTooLarge, err.Error())
return
}
name := fi.Name()
if name == "." || name == "" {
name = "download"
@@ -2064,7 +2202,7 @@ func (f *webdavAuthFS) checkTree(ctx context.Context, name string, p auth.PermMa
func (s *Server) ipFilter(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if s.Allow != "" && !allowedIP(s.Allow, clientIP(r)) {
if s.Allow != "" && !allowedIP(s.Allow, s.clientIP(r)) {
http.Error(w, "forbidden", http.StatusForbidden)
return
}
@@ -2072,6 +2210,33 @@ func (s *Server) ipFilter(next http.Handler) http.Handler {
})
}
func (s *Server) clientIP(r *http.Request) net.IP {
host, _, _ := net.SplitHostPort(r.RemoteAddr)
remote := net.ParseIP(host)
if remote == nil {
remote = net.ParseIP(r.RemoteAddr)
}
if remote == nil || s.TrustedProxies == "" || !allowedIP(s.TrustedProxies, remote) {
return remote
}
// Only a trusted proxy may supply the forwarding chain. Walk right-to-left
// and return the first address that is not itself a trusted proxy.
parts := strings.Split(r.Header.Get("X-Forwarded-For"), ",")
for i := len(parts) - 1; i >= 0; i-- {
ip := net.ParseIP(strings.TrimSpace(parts[i]))
if ip == nil {
continue
}
if !allowedIP(s.TrustedProxies, ip) {
return ip
}
}
if v := net.ParseIP(strings.TrimSpace(r.Header.Get("X-Real-IP"))); v != nil && !allowedIP(s.TrustedProxies, v) {
return v
}
return remote
}
func clientIP(r *http.Request) net.IP {
host, _, err := net.SplitHostPort(r.RemoteAddr)
if err == nil {