From 0a2a23d94361e7e190e9858558b71f1982d90b0c Mon Sep 17 00:00:00 2001 From: niksis02 Date: Fri, 5 Dec 2025 22:56:37 +0400 Subject: [PATCH] fix: Checks that x-amz-decoded-content-length matches the actual payload in unsigned streaming upload Fixes #1676 `x-amz-decoded-content-length` in streaming uploads specifies the number of actual data-payload bytes, with encoding characters removed. If the value does not match the actual payload after decoding, now an `IncompleteBody` error is returned. --- s3api/middlewares/public-bucket.go | 6 +++- s3api/utils/chunk-reader.go | 22 ++++++++---- s3api/utils/unsigned-chunk-reader.go | 14 +++++++- s3err/s3err.go | 6 ++++ tests/integration/group-tests.go | 2 ++ .../unsigned_streaming_payload_trailer.go | 35 +++++++++++++++++++ 6 files changed, 77 insertions(+), 8 deletions(-) diff --git a/s3api/middlewares/public-bucket.go b/s3api/middlewares/public-bucket.go index 60b1576..ba7dce5 100644 --- a/s3api/middlewares/public-bucket.go +++ b/s3api/middlewares/public-bucket.go @@ -79,6 +79,10 @@ func AuthorizePublicBucketAccess(be backend.Backend, s3action string, policyPerm if streamBody { if utils.IsUnsignedStreamingPayload(payloadHash) { + cLength, err := utils.ParseDecodedContentLength(ctx) + if err != nil { + return err + } // stack an unsigned streaming payload reader checksumType, err := utils.ExtractChecksumType(ctx) if err != nil { @@ -87,7 +91,7 @@ func AuthorizePublicBucketAccess(be backend.Backend, s3action string, policyPerm wrapBodyReader(ctx, func(r io.Reader) io.Reader { var cr io.Reader - cr, err = utils.NewUnsignedChunkReader(r, checksumType) + cr, err = utils.NewUnsignedChunkReader(r, checksumType, cLength) return cr }) diff --git a/s3api/utils/chunk-reader.go b/s3api/utils/chunk-reader.go index 76309c6..19773f5 100644 --- a/s3api/utils/chunk-reader.go +++ b/s3api/utils/chunk-reader.go @@ -160,22 +160,32 @@ func IsStreamingPayload(str string) bool { pt == payloadTypeStreamingSignedTrailer } -func NewChunkReader(ctx *fiber.Ctx, r io.Reader, authdata AuthData, region, secret string, date time.Time) (io.Reader, error) { +// ParseDecodedContentLength extracts and validates the +// 'x-amz-decoded-content-length' from fiber context +func ParseDecodedContentLength(ctx *fiber.Ctx) (int64, error) { decContLengthStr := ctx.Get("X-Amz-Decoded-Content-Length") if decContLengthStr == "" { debuglogger.Logf("missing required header 'X-Amz-Decoded-Content-Length'") - return nil, s3err.GetAPIError(s3err.ErrMissingContentLength) + return 0, s3err.GetAPIError(s3err.ErrMissingContentLength) } decContLength, err := strconv.ParseInt(decContLengthStr, 10, 64) - //TODO: not sure if InvalidRequest should be returned in this case if err != nil { debuglogger.Logf("invalid value for 'X-Amz-Decoded-Content-Length': %v", decContLengthStr) - return nil, s3err.GetAPIError(s3err.ErrMissingContentLength) + return 0, s3err.GetAPIError(s3err.ErrMissingContentLength) } if decContLength > maxObjSizeLimit { debuglogger.Logf("the object size exceeds the allowed limit: (size): %v, (limit): %v", decContLength, int64(maxObjSizeLimit)) - return nil, s3err.GetAPIError(s3err.ErrEntityTooLarge) + return 0, s3err.GetAPIError(s3err.ErrEntityTooLarge) + } + + return decContLength, nil +} + +func NewChunkReader(ctx *fiber.Ctx, r io.Reader, authdata AuthData, region, secret string, date time.Time) (io.Reader, error) { + cLength, err := ParseDecodedContentLength(ctx) + if err != nil { + return nil, err } contentSha256 := payloadType(ctx.Get("X-Amz-Content-Sha256")) @@ -192,7 +202,7 @@ func NewChunkReader(ctx *fiber.Ctx, r io.Reader, authdata AuthData, region, secr switch contentSha256 { case payloadTypeStreamingUnsignedTrailer: - return NewUnsignedChunkReader(r, checksumType) + return NewUnsignedChunkReader(r, checksumType, cLength) case payloadTypeStreamingSignedTrailer: return NewSignedChunkReader(r, authdata, region, secret, date, checksumType) case payloadTypeStreamingSigned: diff --git a/s3api/utils/unsigned-chunk-reader.go b/s3api/utils/unsigned-chunk-reader.go index 24e26d1..8e77a4d 100644 --- a/s3api/utils/unsigned-chunk-reader.go +++ b/s3api/utils/unsigned-chunk-reader.go @@ -50,9 +50,13 @@ type UnsignedChunkReader struct { // this data is necessary for 'InvalidChunkSizeError' error // TODO: add 'Chunk' and 'BadChunkSize' in the error chunkSizes []int64 + cLength int64 + // This data is necessary for the decoded content length mismatch error + // TODO: add 'NumberBytesExpected' and 'NumberBytesProvided' in the error + dataRead int64 } -func NewUnsignedChunkReader(r io.Reader, ct checksumType) (*UnsignedChunkReader, error) { +func NewUnsignedChunkReader(r io.Reader, ct checksumType, decContentLength int64) (*UnsignedChunkReader, error) { var hasher hash.Hash var err error if ct != "" { @@ -70,6 +74,7 @@ func NewUnsignedChunkReader(r io.Reader, ct checksumType) (*UnsignedChunkReader, stash: make([]byte, 0), hasher: hasher, chunkSizes: []int64{}, + cLength: decContentLength, }, nil } @@ -104,6 +109,8 @@ func (ucr *UnsignedChunkReader) Read(p []byte) (int, error) { return 0, err } + ucr.dataRead += chunkSize + if chunkSize == 0 { // Stop reading parsing payloads as 0 sized chunk is reached break @@ -146,6 +153,11 @@ func (ucr *UnsignedChunkReader) Read(p []byte) (int, error) { } } + if ucr.cLength != ucr.dataRead { + debuglogger.Logf("number of bytes expected: (%v), number of bytes read: (%v)", ucr.cLength, ucr.dataRead) + return 0, s3err.GetAPIError(s3err.ErrContentLengthMismatch) + } + // Read and validate trailers if err := ucr.readTrailer(); err != nil { debuglogger.Logf("failed to read trailer: %v", err) diff --git a/s3err/s3err.go b/s3err/s3err.go index 70ee11e..8dcc5ed 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -121,6 +121,7 @@ const ( ErrInvalidSHA256Paylod ErrUnsupportedAnonymousSignedStreaming ErrMissingContentLength + ErrContentLengthMismatch ErrInvalidAccessKeyID ErrRequestNotReadyYet ErrMissingDateHeader @@ -520,6 +521,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "You must provide the Content-Length HTTP header.", HTTPStatusCode: http.StatusLengthRequired, }, + ErrContentLengthMismatch: { + Code: "IncompleteBody", + Description: "You did not provide the number of bytes specified by the Content-Length HTTP header", + HTTPStatusCode: http.StatusBadRequest, + }, ErrMissingDateHeader: { Code: "AccessDenied", Description: "AWS authentication requires a valid Date or x-amz-date header.", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 4ee9bff..282a7f3 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1102,6 +1102,7 @@ func TestUnsignedStreaminPayloadTrailer(ts *TestState) { ts.Run(UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch) ts.Run(UnsignedStreamingPayloadTrailer_incomplete_body) ts.Run(UnsignedStreamingPayloadTrailer_invalid_chunk_size) + ts.Run(UnsignedStreamingPayloadTrailer_content_length_payload_size_mismatch) ts.Run(UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme) ts.Run(UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers) ts.Run(UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer) @@ -1756,6 +1757,7 @@ func GetIntTests() IntTests { "UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch": UnsignedStreamingPayloadTrailer_sdk_algo_and_trailer_mismatch, "UnsignedStreamingPayloadTrailer_incomplete_body": UnsignedStreamingPayloadTrailer_incomplete_body, "UnsignedStreamingPayloadTrailer_invalid_chunk_size": UnsignedStreamingPayloadTrailer_invalid_chunk_size, + "UnsignedStreamingPayloadTrailer_content_length_payload_size_mismatch": UnsignedStreamingPayloadTrailer_content_length_payload_size_mismatch, "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme": UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme, "UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers": UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers, "UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer": UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer, diff --git a/tests/integration/unsigned_streaming_payload_trailer.go b/tests/integration/unsigned_streaming_payload_trailer.go index 26a14e5..4de2eb7 100644 --- a/tests/integration/unsigned_streaming_payload_trailer.go +++ b/tests/integration/unsigned_streaming_payload_trailer.go @@ -268,6 +268,41 @@ func UnsignedStreamingPayloadTrailer_invalid_chunk_size(s *S3Conf) error { }) } +func UnsignedStreamingPayloadTrailer_content_length_payload_size_mismatch(s *S3Conf) error { + testName := "UnsignedStreamingPayloadTrailer_content_length_payload_size_mismatch" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + object := "my-object" + for i, test := range []struct { + payload string + cLength int64 + trailer string + }{ + {"b\r\nabcdefghijk\r\n0\r\n\r\n", 5, ""}, + {"b\r\nabcdefghijk\r\n0\r\n\r\n", 200, ""}, + {"a\r\ndummy data\r\n0\r\nx-amz-checksum-crc64nvme:dPVWc2vU1+Q=\r\n\r\n", 128, "crc64nvme"}, + {"a\r\ndummy data\r\n0\r\nx-amz-checksum-sha256:eXuwq/95jXIAr3aF3KeQHt/8Ur8mUA1b2XKCZY7iQVI=\r\n\r\n", 7, "crc64nvme"}, + } { + reqHeaders := map[string]string{ + "x-amz-decoded-content-length": fmt.Sprint(test.cLength), + } + if test.trailer != "" { + reqHeaders["x-amz-trailer"] = fmt.Sprintf("x-amz-checksum-%s", test.trailer) + } + + _, apiErr, err := testUnsignedStreamingPayloadTrailerObjectPut(s, bucket, object, []byte(test.payload), reqHeaders) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + if err := compareS3ApiError(s3err.GetAPIError(s3err.ErrContentLengthMismatch), apiErr); err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + return nil + }) +} + func UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme(s *S3Conf) error { testName := "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {