s3: validate the version-id header used as a filer path segment (#11097)

* s3: reject a version-id header that is not a valid path segment

putToFiler stored the client-supplied Seaweed-X-Amz-Version-Id header
verbatim into object metadata. That value is later read back and used
as a filer path component when building the .versions/v_<id> path, so a
value containing "/", "\" or ".." could steer retention/legal-hold
writes and remote-cache reads outside the object's own bucket tree.

Validate the header with isValidVersionID before storing it, the same
check the versioned read paths already apply, and reject the request
otherwise. Server-set version ids ("null" and generated hex) pass.

Claude-Session: https://claude.ai/code/session_011QqNaxZwnpHgMoAZNp3RkY

* s3: validate a stored version-id before using it as a path

The retention and legal-hold sinks build a .versions/v_<id> path from a
version id read back out of object metadata, and the remote-cache path
builder does the same from either the request or the stored id, without
the isValidVersionID check the other version-id consumers apply. Guard
these so a value that is not a valid path segment falls back to the
regular / unversioned path instead of steering the write or read out of
the bucket tree.

Claude-Session: https://claude.ai/code/session_011QqNaxZwnpHgMoAZNp3RkY
This commit is contained in:
Chris Lu
2026-09-02 11:36:58 -07:00
committed by GitHub
parent 23adeb37e2
commit 9ea52db219
4 changed files with 45 additions and 4 deletions
+3 -1
View File
@@ -3381,7 +3381,9 @@ func (s3a *S3ApiServer) doCacheRemoteObject(ctx context.Context, dir, name strin
}
func (s3a *S3ApiServer) buildVersionedRemoteObjectPath(bucket, object, versionId string) (dir, name string) {
if versionId != "" && versionId != "null" {
// versionId names a .versions file, so a value that is not a valid path
// segment falls back to the unversioned remote path rather than escaping it.
if versionId != "" && versionId != "null" && isValidVersionID(versionId) {
normalizedObject := s3_constants.NormalizeObjectKey(object)
return s3a.bucketDir(bucket) + "/" + normalizedObject + s3_constants.VersionsFolder, s3a.getVersionFileName(versionId)
}
+7 -1
View File
@@ -788,8 +788,14 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader
// Set object owner according to bucket ownership settings.
s3a.setObjectOwnerFromRequest(r, bucket, entry)
// Set version ID if present
// Set version ID if present. It is later used as a filer path segment, so a
// value carrying "/", "\\" or ".." must never be stored.
if versionIdHeader := r.Header.Get(s3_constants.ExtVersionIdKey); versionIdHeader != "" {
if !isValidVersionID(versionIdHeader) {
glog.Warningf("putToFiler: rejecting invalid version ID %q for object %s", versionIdHeader, filePath)
s3a.deleteOrphanedChunks(chunkResult.FileChunks)
return "", s3err.ErrInvalidRequest, SSEResponseMetadata{}
}
entry.Extended[s3_constants.ExtVersionIdKey] = []byte(versionIdHeader)
glog.V(3).Infof("putToFiler: setting version ID %s for object %s", versionIdHeader, filePath)
}
+6 -2
View File
@@ -282,7 +282,9 @@ func (s3a *S3ApiServer) setObjectRetention(bucket, object, versionId string, ret
if entry.Extended != nil {
if versionIdBytes, exists := entry.Extended[s3_constants.ExtVersionIdKey]; exists {
versionId = string(versionIdBytes)
if versionId != "null" {
// A stored version id must be a valid path segment before it can
// name a .versions file; otherwise fall back to the regular object.
if versionId != "null" && isValidVersionID(versionId) {
entryPath = object + ".versions/" + s3a.getVersionFileName(versionId)
}
}
@@ -425,7 +427,9 @@ func (s3a *S3ApiServer) setObjectLegalHold(bucket, object, versionId string, leg
if entry.Extended != nil {
if versionIdBytes, exists := entry.Extended[s3_constants.ExtVersionIdKey]; exists {
versionId = string(versionIdBytes)
if versionId != "null" {
// A stored version id must be a valid path segment before it can
// name a .versions file; otherwise fall back to the regular object.
if versionId != "null" && isValidVersionID(versionId) {
entryPath = object + ".versions/" + s3a.getVersionFileName(versionId)
}
}
@@ -0,0 +1,29 @@
package s3api
import "testing"
func TestBuildVersionedRemoteObjectPathRejectsTraversal(t *testing.T) {
s3a := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets"}}
tests := []struct {
name string
versionId string
wantDir string
wantName string
}{
{"valid version", "opaque_123", "/buckets/mybkt/obj.versions", "v_opaque_123"},
{"traversal drops to unversioned", "v1/../../../buckets/victim/pwn", "/buckets/mybkt", "obj"},
{"backslash drops to unversioned", `v1\..\victim`, "/buckets/mybkt", "obj"},
{"empty is unversioned", "", "/buckets/mybkt", "obj"},
{"null is unversioned", "null", "/buckets/mybkt", "obj"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dir, name := s3a.buildVersionedRemoteObjectPath("mybkt", "obj", tt.versionId)
if dir != tt.wantDir || name != tt.wantName {
t.Errorf("buildVersionedRemoteObjectPath(mybkt, obj, %q) = (%q, %q), want (%q, %q)",
tt.versionId, dir, name, tt.wantDir, tt.wantName)
}
})
}
}