Skip to content

Fix read-and-copy overflow and trim per-chunk work in ChunkedUploadWrapperStream #4570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
martincostello wants to merge 1 commit into from
Closed

Fix read-and-copy overflow and trim per-chunk work in ChunkedUploadWrapperStream #4570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions generator/.DevConfigs/e6d751e3-8e3f-4149-8551-997310abd288.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
}
112 changes: 61 additions & 51 deletions sdk/src/Core/Amazon.Runtime/Internal/Util/ChunkedUploadWrapperStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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)];
}
}

/// <summary>
Expand Down Expand Up @@ -312,10 +329,12 @@ private async Task<int> 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
Expand Down Expand Up @@ -353,21 +372,7 @@ private async Task<int> FillInputBufferAsync(CancellationToken cancellationToken
/// <param name="dataLen"></param>
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)
Expand All @@ -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);
// "<hex size>;chunk-signature=<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<byte>();

// 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)
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -635,14 +632,25 @@ public static long ComputeChunkedContentLength(long originalLength, int signatur
/// <param name="inputBuffer">Content of this chunk</param>
/// <returns>The string to sign for this chunk</returns>
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);
}

/// <summary>
/// Same as <see cref="BuildChunkedStringToSign(string, string, string, string, int, byte[])"/> but hashes only the
/// <paramref name="count"/> bytes of chunk data starting at <paramref name="offset"/>, so a partially filled
/// chunk buffer can be signed without copying it.
/// </summary>
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);
}

Expand Down Expand Up @@ -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
Expand Down
105 changes: 105 additions & 0 deletions sdk/test/UnitTests/Custom/Runtime/ChunkedUploadWrapperStreamTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -265,5 +266,109 @@ public void ChecksumAndOtherTrailingHeaders()
Assert.AreEqual(expectedContent.Length, wrappedStream.Length);
Assert.AreEqual(expectedContent, actualContent);
}

/// <summary>
/// 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.
/// </summary>
[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<int> { 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<string>();
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);
}

/// <summary>
/// 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.
/// </summary>
[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();
}
}

/// <summary>
/// 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.
/// </summary>
private static byte[] ReassembleChunks(byte[] chunked, out List<int> chunkSizes, out List<string> chunkSignatures)
{
chunkSizes = new List<int>();
chunkSignatures = new List<string>();
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.");
}

}
}