From 01c2f2c17b4555bcefd98316b59b34b04c518cb3 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 27 Oct 2026 10:25:03 +0000 Subject: [PATCH] Fix read-and-copy overflow and trim per-chunk work in ChunkedUploadWrapperStream The read-and-copy fill loops (used when the wrapped stream encrypts) always asked the wrapped stream for a full wrapped-stream-buffer-size read, even when fewer bytes fit in the remaining space of the chunk buffer. With a BufferSize that does not divide DefaultChunkSize (or exceeds it) the Buffer.BlockCopy into the chunk buffer throws mid-upload. Each read is now clamped to the space left in the chunk, and the interim read buffer is allocated once per stream, sized to at most a chunk, instead of once per fill. Per-chunk allocations are reduced at the same time: - The short final chunk is no longer copied into an exact-size array. SigV4 hashes only the bytes read through a new internal string-to-sign overload that takes an offset and count (the public overload keeps its contract and delegates to it), and SigV4a signs a MemoryStream slice. - The chunk header is encoded straight into the output buffer instead of going through a StringBuilder, a byte[] header and a byte[] trailer per chunk. - The SigV4 signing key is captured once per stream instead of being cloned by GetSigningKey() for every chunk and again for the trailer. - The empty-body hash in the string to sign uses the existing constant. Tests cover a multi-chunk payload round-tripping with every chunk signature (including the terminating chunk) recomputed independently from exact-size copies via the public string-to-sign helper, and an encrypting stream read with a buffer larger than a chunk, which fails with an ArgumentException before this change. --- .../e6d751e3-8e3f-4149-8551-997310abd288.json | 9 ++ .../Util/ChunkedUploadWrapperStream.cs | 112 ++++++++++-------- .../ChunkedUploadWrapperStreamTests.cs | 105 ++++++++++++++++ 3 files changed, 175 insertions(+), 51 deletions(-) create mode 100644 generator/.DevConfigs/e6d751e3-8e3f-4149-8551-997310abd288.json diff --git a/generator/.DevConfigs/e6d751e3-8e3f-4149-8551-997310abd288.json b/generator/.DevConfigs/e6d751e3-8e3f-4149-8551-997310abd288.json new file mode 100644 index 000000000000..1777e692cce3 --- /dev/null +++ b/generator/.DevConfigs/e6d751e3-8e3f-4149-8551-997310abd288.json @@ -0,0 +1,9 @@ +{ + "core": { + "changeLogMessages": [ + "Fixed a buffer overflow in ChunkedUploadWrapperStream when an encrypting stream is uploaded with a buffer size that does not divide the chunk size, and reduced the per-chunk allocations of chunked uploads." + ], + "type": "patch", + "updateMinimum": false + } +} diff --git a/sdk/src/Core/Amazon.Runtime/Internal/Util/ChunkedUploadWrapperStream.cs b/sdk/src/Core/Amazon.Runtime/Internal/Util/ChunkedUploadWrapperStream.cs index f016e1f080b9..0f6a40941980 100644 --- a/sdk/src/Core/Amazon.Runtime/Internal/Util/ChunkedUploadWrapperStream.cs +++ b/sdk/src/Core/Amazon.Runtime/Internal/Util/ChunkedUploadWrapperStream.cs @@ -47,7 +47,10 @@ public class ChunkedUploadWrapperStream : WrapperStream private const string TRAILING_HEADER_SIGNATURE_KEY = "x-amz-trailer-signature"; private const string TRAILING_HEADER_STRING_TO_SIGN_PREFIX = "AWS4-HMAC-SHA256-TRAILER"; - private byte[] _inputBuffer; + private readonly byte[] _inputBuffer; + + // SigV4 signing key, captured once since AWS4SigningResult.GetSigningKey() returns a copy. + private readonly byte[] _signingKey; private readonly byte[] _outputBuffer; private int _outputBufferPos = -1; @@ -56,6 +59,9 @@ public class ChunkedUploadWrapperStream : WrapperStream private readonly int _wrappedStreamBufferSize; private bool _wrappedStreamConsumed; + // Interim buffer for the ReadAndCopy strategy, reused across chunks. + private readonly byte[] _readAndCopyBuffer; + private CoreChecksumAlgorithm _trailingChecksum; private HashAlgorithm _hashAlgorithm; private readonly CachedChecksum _cachedChecksum; @@ -100,10 +106,12 @@ internal ChunkedUploadWrapperStream(Stream stream, int wrappedStreamBufferSize, HeaderSigningResult = headerSigningResult; PreviousChunkSignature = headerSigningResult?.Signature; + if (headerSigningResult is AWS4SigningResult v4SigningResult) + { + _signingKey = v4SigningResult.GetSigningKey(); + } _wrappedStreamBufferSize = wrappedStreamBufferSize; - _inputBuffer = new byte[DefaultChunkSize]; - _outputBuffer = new byte[CalculateChunkHeaderLength(DefaultChunkSize, HeaderSigningResult is AWS4aSigningResult ? V4A_SIGNATURE_LENGTH : V4_SIGNATURE_LENGTH)]; // header+data // if the wrapped stream implements encryption, switch to a read-and-copy // strategy for filling the chunk buffer @@ -119,6 +127,15 @@ internal ChunkedUploadWrapperStream(Stream stream, int wrappedStreamBufferSize, if (encryptionStream != null) _readStrategy = ReadStrategy.ReadAndCopy; + + _inputBuffer = new byte[DefaultChunkSize]; + _outputBuffer = new byte[CalculateChunkHeaderLength(DefaultChunkSize, HeaderSigningResult is AWS4aSigningResult ? V4A_SIGNATURE_LENGTH : V4_SIGNATURE_LENGTH)]; // header+data + + if (_readStrategy == ReadStrategy.ReadAndCopy) + { + // reads are clamped to the chunk buffer, so the interim buffer never needs to be larger than it + _readAndCopyBuffer = new byte[Math.Min(wrappedStreamBufferSize, DefaultChunkSize)]; + } } /// @@ -312,10 +329,12 @@ private async Task FillInputBufferAsync(CancellationToken cancellationToken } else { - var readBuffer = new byte[_wrappedStreamBufferSize]; + var readBuffer = _readAndCopyBuffer; while (inputBufferPos < _inputBuffer.Length && !_wrappedStreamConsumed) { - var bytesRead = await BaseStream.ReadAsync(readBuffer, 0, _wrappedStreamBufferSize, cancellationToken).ConfigureAwait(false); + // never read more than the chunk buffer can still hold + var bytesToRead = Math.Min(_wrappedStreamBufferSize, _inputBuffer.Length - inputBufferPos); + var bytesRead = await BaseStream.ReadAsync(readBuffer, 0, bytesToRead, cancellationToken).ConfigureAwait(false); if (bytesRead == 0) _wrappedStreamConsumed = true; else @@ -353,21 +372,7 @@ private async Task FillInputBufferAsync(CancellationToken cancellationToken /// private void ConstructOutputBufferChunk(int dataLen) { - // if the input wasn't sufficient to fill the buffer, size it - // down to make the subseqent hashing/computations easier since - // they don't take any length arguments - if (dataLen > 0 && dataLen < _inputBuffer.Length) - { - var temp = new byte[dataLen]; - Buffer.BlockCopy(_inputBuffer, 0, temp, 0, dataLen); - _inputBuffer = temp; - } - var isFinalDataChunk = dataLen == 0; - var chunkHeader = new StringBuilder(); - - // variable-length size of the embedded chunk data in hex - chunkHeader.Append(dataLen.ToString("X", CultureInfo.InvariantCulture)); string chunkSignature = ""; if (HeaderSigningResult is AWS4aSigningResult v4aHeaderSigningResult) @@ -378,59 +383,51 @@ private void ConstructOutputBufferChunk(int dataLen) } else { - chunkSignature = Sigv4aSigner.SignChunk(new MemoryStream(_inputBuffer), PreviousChunkSignature, v4aHeaderSigningResult); + // only the dataLen bytes that were read belong to this chunk + chunkSignature = Sigv4aSigner.SignChunk(new MemoryStream(_inputBuffer, 0, dataLen), PreviousChunkSignature, v4aHeaderSigningResult); } } else if (HeaderSigningResult is AWS4SigningResult v4HeaderSingingResult) // SigV4 { var chunkStringToSign = BuildChunkedStringToSign(CHUNK_STRING_TO_SIGN_PREFIX, v4HeaderSingingResult.ISO8601DateTime, - v4HeaderSingingResult.Scope, PreviousChunkSignature, dataLen, _inputBuffer); + v4HeaderSingingResult.Scope, PreviousChunkSignature, _inputBuffer, 0, dataLen); - chunkSignature = AWSSDKUtils.ToHex(AWS4Signer.SignBlob(v4HeaderSingingResult.GetSigningKey(), chunkStringToSign), true); + chunkSignature = AWSSDKUtils.ToHex(AWS4Signer.SignBlob(_signingKey, chunkStringToSign), true); } // For Sigv4a the chunk signature must be padded when being appended to the chunk metadata, // but not when being used as the input for the next chunk PreviousChunkSignature = chunkSignature; - if (HeaderSigningResult is AWS4aSigningResult) - { - chunkHeader.Append(CHUNK_SIGNATURE_HEADER + chunkSignature.PadRight(V4A_SIGNATURE_LENGTH, '*')); - } - else // SigV4 - { - chunkHeader.Append(CHUNK_SIGNATURE_HEADER + chunkSignature); - } + var paddedChunkSignature = HeaderSigningResult is AWS4aSigningResult + ? chunkSignature.PadRight(V4A_SIGNATURE_LENGTH, '*') + : chunkSignature; // If we're sending a trailing checksum, update the rolling checksum with this chunk's raw data if (_hashAlgorithm != null) { _hashAlgorithm.TransformBlock(_inputBuffer, 0, dataLen, _inputBuffer, 0); } - chunkHeader.Append(STREAM_NEWLINE); + // ";chunk-signature=\r\n": all ASCII, so it is encoded straight into the output buffer + var chunkHeader = dataLen.ToString("X", CultureInfo.InvariantCulture) + CHUNK_SIGNATURE_HEADER + paddedChunkSignature + STREAM_NEWLINE; try { - var header = Encoding.UTF8.GetBytes(chunkHeader.ToString()); - var trailer = ArrayEx.Empty(); - - // Append a trailing CRLF unless this is the final data chunk and there are trailing headers - if (!(isFinalDataChunk && _trailingHeaders?.Count > 0)) - { - trailer = Encoding.UTF8.GetBytes(STREAM_NEWLINE); - } - - var writePos = 0; - Buffer.BlockCopy(header, 0, _outputBuffer, writePos, header.Length); - writePos += header.Length; + var writePos = Encoding.UTF8.GetBytes(chunkHeader, 0, chunkHeader.Length, _outputBuffer, 0); if (dataLen > 0) { Buffer.BlockCopy(_inputBuffer, 0, _outputBuffer, writePos, dataLen); writePos += dataLen; } - Buffer.BlockCopy(trailer, 0, _outputBuffer, writePos, trailer.Length); + + // Append a trailing CRLF unless this is the final data chunk and there are trailing headers + if (!(isFinalDataChunk && _trailingHeaders?.Count > 0)) + { + _outputBuffer[writePos++] = (byte)'\r'; + _outputBuffer[writePos++] = (byte)'\n'; + } _outputBufferPos = 0; - _outputBufferDataLen = header.Length + dataLen + trailer.Length; + _outputBufferDataLen = writePos; } catch (Exception e) { @@ -475,7 +472,7 @@ private string ConstructSignedTrailersChunk() PreviousChunkSignature + "\n" + AWSSDKUtils.ToHex(AWS4Signer.ComputeHash(canonicalizedTrailingHeaders), true); - chunkSignature = AWSSDKUtils.ToHex(AWS4Signer.SignBlob(((AWS4SigningResult)HeaderSigningResult).GetSigningKey(), chunkStringToSign), true); + chunkSignature = AWSSDKUtils.ToHex(AWS4Signer.SignBlob(_signingKey, chunkStringToSign), true); } else // SigV4a { @@ -635,14 +632,25 @@ public static long ComputeChunkedContentLength(long originalLength, int signatur /// Content of this chunk /// The string to sign for this chunk public static string BuildChunkedStringToSign(string prefix, string dateTime, string scope, string previousSignature, int dataLength, byte[] inputBuffer) + { + // The whole buffer is hashed when there is data, matching the historical behavior of this overload. + return BuildChunkedStringToSign(prefix, dateTime, scope, previousSignature, inputBuffer, 0, dataLength > 0 ? inputBuffer.Length : 0); + } + + /// + /// Same as but hashes only the + /// bytes of chunk data starting at , so a partially filled + /// chunk buffer can be signed without copying it. + /// + internal static string BuildChunkedStringToSign(string prefix, string dateTime, string scope, string previousSignature, byte[] inputBuffer, int offset, int count) { return prefix + "\n" + dateTime + "\n" + scope + "\n" + previousSignature + "\n" + - AWSSDKUtils.ToHex(AWS4Signer.ComputeHash(""), true) + "\n" + - (dataLength > 0 - ? AWSSDKUtils.ToHex(AWS4Signer.ComputeHash(inputBuffer), true) + AWS4Signer.EmptyBodySha256 + "\n" + + (count > 0 + ? AWSSDKUtils.ToHex(CryptoUtilFactory.CryptoInstance.ComputeSHA256Hash(inputBuffer, offset, count), true) : AWS4Signer.EmptyBodySha256); } @@ -691,10 +699,12 @@ private int FillInputBuffer() } else { - var readBuffer = new byte[_wrappedStreamBufferSize]; + var readBuffer = _readAndCopyBuffer; while (inputBufferPos < _inputBuffer.Length && !_wrappedStreamConsumed) { - var bytesRead = BaseStream.Read(readBuffer, 0, _wrappedStreamBufferSize); + // never read more than the chunk buffer can still hold + var bytesToRead = Math.Min(_wrappedStreamBufferSize, _inputBuffer.Length - inputBufferPos); + var bytesRead = BaseStream.Read(readBuffer, 0, bytesToRead); if (bytesRead == 0) _wrappedStreamConsumed = true; else diff --git a/sdk/test/UnitTests/Custom/Runtime/ChunkedUploadWrapperStreamTests.cs b/sdk/test/UnitTests/Custom/Runtime/ChunkedUploadWrapperStreamTests.cs index d601dc636d32..ccec49129a46 100644 --- a/sdk/test/UnitTests/Custom/Runtime/ChunkedUploadWrapperStreamTests.cs +++ b/sdk/test/UnitTests/Custom/Runtime/ChunkedUploadWrapperStreamTests.cs @@ -16,6 +16,7 @@ using Amazon.Runtime.Internal; using Amazon.Runtime.Internal.Auth; using Amazon.Runtime.Internal.Util; +using Amazon.Util; using Microsoft.VisualStudio.TestTools.UnitTesting; using System; using System.Collections.Generic; @@ -265,5 +266,109 @@ public void ChecksumAndOtherTrailingHeaders() Assert.AreEqual(expectedContent.Length, wrappedStream.Length); Assert.AreEqual(expectedContent, actualContent); } + + /// + /// A payload that is not a multiple of the chunk size produces a full chunk, a short chunk and the terminator, + /// and the data round-trips unchanged. + /// + [TestMethod] + public void MultiChunkPayloadRoundTrips() + { + var data = new byte[ChunkedUploadWrapperStream.DefaultChunkSize + 12345]; + new Random(42).NextBytes(data); + var headerSigningResult = new AWS4SigningResult(_accessKey, _secretKey, _fixedSigningTimestamp, "", "", _signingKey, _headerSignature); + var wrappedStream = new ChunkedUploadWrapperStream(new MemoryStream(data), 8192, headerSigningResult); + + var output = ReadAll(wrappedStream); + + Assert.AreEqual(wrappedStream.Length, output.Length); + CollectionAssert.AreEqual(data, ReassembleChunks(output, out var chunkSizes, out var chunkSignatures)); + CollectionAssert.AreEqual(new List { ChunkedUploadWrapperStream.DefaultChunkSize, 12345 }, chunkSizes); + + // Each chunk signature must cover exactly that chunk's bytes: recompute them independently from + // exact-size copies with the public string-to-sign helper and chain them from the header signature. + var expectedSignatures = new List(); + var previousSignature = headerSigningResult.Signature; + var offset = 0; + foreach (var size in new[] { chunkSizes[0], chunkSizes[1], 0 }) + { + var chunk = new byte[size]; + Buffer.BlockCopy(data, offset, chunk, 0, size); + offset += size; + var stringToSign = ChunkedUploadWrapperStream.BuildChunkedStringToSign("AWS4-HMAC-SHA256-PAYLOAD", + headerSigningResult.ISO8601DateTime, headerSigningResult.Scope, previousSignature, size, chunk); + previousSignature = AWSSDKUtils.ToHex(AWS4Signer.SignBlob(headerSigningResult.GetSigningKey(), stringToSign), true); + expectedSignatures.Add(previousSignature); + } + CollectionAssert.AreEqual(expectedSignatures, chunkSignatures); + } + + /// + /// An encrypting wrapped stream uses the read-and-copy strategy. A read buffer larger than the chunk buffer + /// must be clamped to the space left in the chunk, otherwise the copy overflows. + /// + [TestMethod] + public void EncryptedStreamWithLargeReadBufferDoesNotOverflow() + { + var data = new byte[ChunkedUploadWrapperStream.DefaultChunkSize + 1000]; + new Random(7).NextBytes(data); + var encryptedStream = new AESEncryptionPutObjectStream(new MemoryStream(data), new byte[32], new byte[16]); + var headerSigningResult = new AWS4SigningResult(_accessKey, _secretKey, _fixedSigningTimestamp, "", "", _signingKey, _headerSignature); + var wrappedStream = new ChunkedUploadWrapperStream(encryptedStream, ChunkedUploadWrapperStream.DefaultChunkSize * 2, headerSigningResult); + + var output = ReadAll(wrappedStream); + + Assert.AreEqual(wrappedStream.Length, output.Length); + var cipherText = ReassembleChunks(output, out _, out _); + Assert.AreEqual(encryptedStream.Length, cipherText.Length); + } + + private static byte[] ReadAll(Stream stream) + { + using (var output = new MemoryStream()) + { + stream.CopyTo(output); + return output.ToArray(); + } + } + + /// + /// Parses aws-chunked content (without trailing headers) back into the original payload, also returning the + /// size and signature of every chunk including the terminating zero-length chunk. + /// + private static byte[] ReassembleChunks(byte[] chunked, out List chunkSizes, out List chunkSignatures) + { + chunkSizes = new List(); + chunkSignatures = new List(); + var payload = new MemoryStream(); + var position = 0; + while (true) + { + var headerEnd = IndexOf(chunked, (byte)'\n', position); + var header = Encoding.ASCII.GetString(chunked, position, headerEnd - position - 1); // without \r + var size = Convert.ToInt32(header.Substring(0, header.IndexOf(';')), 16); + chunkSignatures.Add(header.Substring(header.IndexOf('=') + 1)); + position = headerEnd + 1; + if (size == 0) + { + break; + } + chunkSizes.Add(size); + payload.Write(chunked, position, size); + position += size + 2; // data + \r\n + } + return payload.ToArray(); + } + + private static int IndexOf(byte[] data, byte value, int start) + { + for (var i = start; i < data.Length; i++) + { + if (data[i] == value) + return i; + } + throw new InvalidOperationException("Malformed chunked content."); + } + } }