mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-05-29 21:20:21 +00:00
dd1b428789
* s3,iceberg: reject `..`/NUL in URL path vars Both gateway routers use mux.NewRouter().SkipClean(true), so a request like `GET /bucket-A/../evil-bucket/key` survives routing as bucket=bucket-A, object=../evil-bucket/key. The captured key is then joined into a filer path; util.JoinPath / path.Join collapse the `..` server-side and the read lands in evil-bucket. With auth on, IAM still authorizes against bucket-A (the mux var), so policy is evaluated against the wrong target. Add a middleware on the S3 bucket subrouter and the Iceberg REST router that rejects any `.`, `..`, NUL, or — for single-segment slots — embedded slash in the captured path vars before any handler runs. NormalizeObjectKey already folds `\` to `/` and decoding happens in mux, so `%2e%2e` and `..\` are caught. * s3,iceberg: reject empty captured vars and empty namespace parts Comma-ok the var lookup so we only check captured slots, then treat an empty captured value as a rejection on its own — downstream path.Join would otherwise collapse it and let the next segment pick the bucket. For iceberg, also reject empty parts after splitting the namespace on \x1F so leading/trailing/consecutive unit separators (which parseNamespace silently folds out) don't let distinct route values collapse to the same parsed namespace. Register loggingMiddleware before validateRequestPath on the iceberg router so rejected requests still produce an audit-log line.
88 lines
3.1 KiB
Go
88 lines
3.1 KiB
Go
package s3api
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/gorilla/mux"
|
|
)
|
|
|
|
func TestValidateRequestPath_RejectsTraversal(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
// rawPath is sent as the Request-URI; net/http.NewRequest does not
|
|
// rewrite the path, so `..` segments survive into mux when the router
|
|
// is built with SkipClean(true) — matching the production setup in
|
|
// weed/command/s3.go.
|
|
rawPath string
|
|
wantCode int
|
|
}{
|
|
{"clean path passes", "/bucket-a/folder/file.txt", http.StatusOK},
|
|
{"bucket only passes", "/bucket-a", http.StatusOK},
|
|
{"trailing slash passes", "/bucket-a/folder/", http.StatusOK},
|
|
|
|
{"leading dotdot rejected", "/bucket-a/../evil-bucket/test.txt", http.StatusBadRequest},
|
|
{"nested dotdot rejected", "/bucket-a/good/../evil/test.txt", http.StatusBadRequest},
|
|
{"backslash dotdot rejected", "/bucket-a/..\\evil\\test.txt", http.StatusBadRequest},
|
|
{"percent-encoded dotdot rejected", "/bucket-a/%2e%2e/evil/test.txt", http.StatusBadRequest},
|
|
{"bare dot object rejected", "/bucket-a/./evil/test.txt", http.StatusBadRequest},
|
|
{"dotdot bucket rejected", "/../buckets/evil", http.StatusBadRequest},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
router := mux.NewRouter().SkipClean(true)
|
|
sub := router.PathPrefix("/{bucket}").Subrouter()
|
|
sub.Use(validateRequestPath)
|
|
handlerCalled := false
|
|
pass := func(w http.ResponseWriter, r *http.Request) {
|
|
handlerCalled = true
|
|
w.WriteHeader(http.StatusOK)
|
|
}
|
|
// Mirror the production routes: /{bucket}/{object:(?s).+} for
|
|
// object-scoped requests, bare /{bucket} for bucket-scoped ones.
|
|
sub.Path("/{object:(?s).+}").HandlerFunc(pass)
|
|
sub.Path("").HandlerFunc(pass)
|
|
|
|
req := httptest.NewRequest(http.MethodGet, tt.rawPath, nil)
|
|
rr := httptest.NewRecorder()
|
|
router.ServeHTTP(rr, req)
|
|
|
|
if rr.Code != tt.wantCode {
|
|
t.Fatalf("path %q: got status %d, want %d (body=%q)", tt.rawPath, rr.Code, tt.wantCode, rr.Body.String())
|
|
}
|
|
if tt.wantCode == http.StatusBadRequest && handlerCalled {
|
|
t.Fatalf("path %q: inner handler reached despite rejection", tt.rawPath)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// Defense-in-depth: a future router or middleware that captures the {bucket}
|
|
// or {object} mux var as an empty string must still be rejected, even though
|
|
// mux's default `[^/]+` regex won't match an empty segment from a real URL.
|
|
func TestValidateRequestPath_RejectsEmptyCapturedVars(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
vars map[string]string
|
|
}{
|
|
{"empty bucket", map[string]string{"bucket": "", "object": "key"}},
|
|
{"empty object", map[string]string{"bucket": "bucket-a", "object": ""}},
|
|
}
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
handlerCalled := false
|
|
h := validateRequestPath(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
handlerCalled = true
|
|
}))
|
|
req := mux.SetURLVars(httptest.NewRequest(http.MethodGet, "/", nil), tt.vars)
|
|
rr := httptest.NewRecorder()
|
|
h.ServeHTTP(rr, req)
|
|
if handlerCalled {
|
|
t.Fatalf("vars %v: inner handler reached despite empty capture", tt.vars)
|
|
}
|
|
})
|
|
}
|
|
}
|