security: harden WebDAV overwrite and multi-root routing
This commit is contained in:
@@ -800,9 +800,28 @@ func (s *Server) webdav(w http.ResponseWriter, r *http.Request) {
|
|||||||
inner = &webdavStorageFS{store: s.Store, removeMode: s.RemoveMode}
|
inner = &webdavStorageFS{store: s.Store, removeMode: s.RemoveMode}
|
||||||
}
|
}
|
||||||
fs := &webdavAuthFS{inner: inner, account: account(r, s)}
|
fs := &webdavAuthFS{inner: inner, account: account(r, s)}
|
||||||
|
if (r.Method == "COPY" || r.Method == "MOVE") && !sameWebDAVDestinationHost(r) {
|
||||||
|
writeJSONError(w, http.StatusBadRequest, "invalid destination")
|
||||||
|
return
|
||||||
|
}
|
||||||
(&webdav.Handler{FileSystem: fs}).ServeHTTP(w, r)
|
(&webdav.Handler{FileSystem: fs}).ServeHTTP(w, r)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func sameWebDAVDestinationHost(r *http.Request) bool {
|
||||||
|
dest := strings.TrimSpace(r.Header.Get("Destination"))
|
||||||
|
if dest == "" {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
u, err := url.Parse(dest)
|
||||||
|
if err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if u.Host == "" {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
return strings.EqualFold(u.Host, r.Host)
|
||||||
|
}
|
||||||
|
|
||||||
type webdavStorageFS struct {
|
type webdavStorageFS struct {
|
||||||
store storage.Storage
|
store storage.Storage
|
||||||
removeMode int
|
removeMode int
|
||||||
@@ -1306,6 +1325,11 @@ func (f *webdavAuthFS) Copy(ctx context.Context, name, dest string, opts *webdav
|
|||||||
if err := f.allow(parent, auth.Write); err != nil {
|
if err := f.allow(parent, auth.Write); err != nil {
|
||||||
return false, err
|
return false, err
|
||||||
}
|
}
|
||||||
|
if opts == nil || !opts.NoOverwrite {
|
||||||
|
if err := f.authorizeOverwrite(ctx, dest); err != nil {
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
}
|
||||||
return f.inner.Copy(ctx, name, dest, opts)
|
return f.inner.Copy(ctx, name, dest, opts)
|
||||||
}
|
}
|
||||||
func (f *webdavAuthFS) Move(ctx context.Context, name, dest string, opts *webdav.MoveOptions) (bool, error) {
|
func (f *webdavAuthFS) Move(ctx context.Context, name, dest string, opts *webdav.MoveOptions) (bool, error) {
|
||||||
@@ -1322,9 +1346,31 @@ func (f *webdavAuthFS) Move(ctx context.Context, name, dest string, opts *webdav
|
|||||||
if err := f.allow(parent, auth.Write); err != nil {
|
if err := f.allow(parent, auth.Write); err != nil {
|
||||||
return false, err
|
return false, err
|
||||||
}
|
}
|
||||||
|
if opts == nil || !opts.NoOverwrite {
|
||||||
|
if err := f.authorizeOverwrite(ctx, dest); err != nil {
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
}
|
||||||
return f.inner.Move(ctx, name, dest, opts)
|
return f.inner.Move(ctx, name, dest, opts)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (f *webdavAuthFS) authorizeOverwrite(ctx context.Context, dest string) error {
|
||||||
|
info, err := f.inner.Stat(ctx, dest)
|
||||||
|
if err != nil {
|
||||||
|
if os.IsNotExist(err) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if err := f.allow(dest, auth.Delete); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if info.IsDir {
|
||||||
|
return f.checkTree(ctx, dest, auth.Delete)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func (f *webdavAuthFS) checkTree(ctx context.Context, name string, p auth.PermMask) error {
|
func (f *webdavAuthFS) checkTree(ctx context.Context, name string, p auth.PermMask) error {
|
||||||
info, err := f.inner.Stat(ctx, name)
|
info, err := f.inner.Stat(ctx, name)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package api
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -37,6 +38,42 @@ func TestWebDAVAuthFSCheckTree(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestWebDAVAuthFSOverwriteRequiresDelete(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
if err := os.WriteFile(filepath.Join(root, "src.txt"), []byte("src"), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(root, "dst.txt"), []byte("dst"), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
account := &auth.Account{Mask: auth.Write}
|
||||||
|
fs := &webdavAuthFS{inner: webdav.LocalFileSystem(root), account: account}
|
||||||
|
if err := fs.authorizeOverwrite(context.Background(), "/dst.txt"); err == nil {
|
||||||
|
t.Fatal("expected overwrite authorization to require delete permission")
|
||||||
|
}
|
||||||
|
account.Mask = auth.Delete
|
||||||
|
if err := fs.authorizeOverwrite(context.Background(), "/dst.txt"); err != nil {
|
||||||
|
t.Fatalf("delete permission should authorize overwrite: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSameWebDAVDestinationHost(t *testing.T) {
|
||||||
|
req := httptest.NewRequest("MOVE", "/webdav/src.txt", nil)
|
||||||
|
req.Host = "example.test"
|
||||||
|
req.Header.Set("Destination", "http://other.test/webdav/dst.txt")
|
||||||
|
if sameWebDAVDestinationHost(req) {
|
||||||
|
t.Fatal("cross-host destination should be rejected")
|
||||||
|
}
|
||||||
|
req.Header.Set("Destination", "http://example.test/webdav/dst.txt")
|
||||||
|
if !sameWebDAVDestinationHost(req) {
|
||||||
|
t.Fatal("same-host absolute destination should be accepted")
|
||||||
|
}
|
||||||
|
req.Header.Set("Destination", "/webdav/dst.txt")
|
||||||
|
if !sameWebDAVDestinationHost(req) {
|
||||||
|
t.Fatal("relative destination should be accepted")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestWebDAVStorageFSCopyAndMove(t *testing.T) {
|
func TestWebDAVStorageFSCopyAndMove(t *testing.T) {
|
||||||
root1 := t.TempDir()
|
root1 := t.TempDir()
|
||||||
root2 := t.TempDir()
|
root2 := t.TempDir()
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package storage
|
|||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
@@ -23,15 +24,9 @@ func NewMulti(paths []string) Multi {
|
|||||||
if name == "." || name == string(filepath.Separator) || name == "" {
|
if name == "." || name == string(filepath.Separator) || name == "" {
|
||||||
name = "root"
|
name = "root"
|
||||||
}
|
}
|
||||||
duplicate := false
|
base := name
|
||||||
for _, existing := range m.Names {
|
for i := 2; containsString(m.Names, name); i++ {
|
||||||
if existing == name {
|
name = base + "-" + strconv.Itoa(i)
|
||||||
duplicate = true
|
|
||||||
break
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if duplicate {
|
|
||||||
continue
|
|
||||||
}
|
}
|
||||||
m.Roots = append(m.Roots, Local{Root: abs})
|
m.Roots = append(m.Roots, Local{Root: abs})
|
||||||
m.Names = append(m.Names, name)
|
m.Names = append(m.Names, name)
|
||||||
@@ -39,6 +34,15 @@ func NewMulti(paths []string) Multi {
|
|||||||
return m
|
return m
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func containsString(items []string, want string) bool {
|
||||||
|
for _, item := range items {
|
||||||
|
if item == want {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
func (m Multi) isVirtualRoot(p string) bool { return strings.Trim(p, "/") == "" }
|
func (m Multi) isVirtualRoot(p string) bool { return strings.Trim(p, "/") == "" }
|
||||||
|
|
||||||
func (m Multi) FindRoot(p string) (string, string, bool) {
|
func (m Multi) FindRoot(p string) (string, string, bool) {
|
||||||
|
|||||||
@@ -0,0 +1,26 @@
|
|||||||
|
package storage
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestNewMultiKeepsDuplicateBasenames(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
first := filepath.Join(root, "same")
|
||||||
|
second := filepath.Join(root, "other", "same")
|
||||||
|
if err := os.MkdirAll(first, 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.MkdirAll(second, 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
m := NewMulti([]string{first, second})
|
||||||
|
if len(m.Roots) != 2 || len(m.Names) != 2 {
|
||||||
|
t.Fatalf("duplicate basename dropped: roots=%d names=%v", len(m.Roots), m.Names)
|
||||||
|
}
|
||||||
|
if m.Names[0] != "same" || m.Names[1] != "same-2" {
|
||||||
|
t.Fatalf("unexpected virtual names: %v", m.Names)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user