diff --git a/sdk/storage/azure-storage-blob-batch/CHANGELOG.md b/sdk/storage/azure-storage-blob-batch/CHANGELOG.md index 7ea3e9312a81f..8cf4ed864b6b5 100644 --- a/sdk/storage/azure-storage-blob-batch/CHANGELOG.md +++ b/sdk/storage/azure-storage-blob-batch/CHANGELOG.md @@ -7,6 +7,10 @@ ### Breaking Changes ### Bugs Fixed +- Fixed a header-injection issue where carriage-return (`\r`) or line-feed (`\n`) characters in a batch operation's + inner request header names or values (for example, a tag condition supplied via + `BlobRequestConditions.setTagsConditions`) were serialized into the multipart batch body without validation. Such + characters are now rejected with an `IllegalArgumentException` before serialization. ### Other Changes diff --git a/sdk/storage/azure-storage-blob-batch/assets.json b/sdk/storage/azure-storage-blob-batch/assets.json index 66f69be89ebbf..8a6e9ad09dab7 100644 --- a/sdk/storage/azure-storage-blob-batch/assets.json +++ b/sdk/storage/azure-storage-blob-batch/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "java", "TagPrefix": "java/storage/azure-storage-blob-batch", - "Tag": "java/storage/azure-storage-blob-batch_606ab979e6" + "Tag": "java/storage/azure-storage-blob-batch_1e28437cd5" } diff --git a/sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/BlobBatchOperationInfo.java b/sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/BlobBatchOperationInfo.java index 6c46ef97f1368..533c7f3b31f46 100644 --- a/sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/BlobBatchOperationInfo.java +++ b/sdk/storage/azure-storage-blob-batch/src/main/java/com/azure/storage/blob/batch/BlobBatchOperationInfo.java @@ -6,6 +6,7 @@ import com.azure.core.annotation.Immutable; import com.azure.core.http.HttpRequest; import com.azure.core.util.CoreUtils; +import com.azure.core.util.logging.ClientLogger; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; @@ -20,6 +21,7 @@ */ @Immutable final class BlobBatchOperationInfo { + private static final ClientLogger LOGGER = new ClientLogger(BlobBatchOperationInfo.class); private static final String X_MS_VERSION = "x-ms-version"; private static final String BATCH_OPERATION_CONTENT_TYPE = "Content-Type: application/http"; private static final String BATCH_OPERATION_CONTENT_TRANSFER_ENCODING = "Content-Transfer-Encoding: binary"; @@ -102,7 +104,8 @@ void addBatchOperation(BlobBatchOperationResponse> batchOperation, HttpRequest request.getHeaders() .stream() .filter(header -> !X_MS_VERSION.equalsIgnoreCase(header.getName())) - .forEach(header -> appendWithNewline(batchRequestBuilder, header.getName() + ": " + header.getValue())); + .forEach(header -> appendWithNewline(batchRequestBuilder, + validateHeader(header.getName()) + ": " + validateHeader(header.getValue()))); batchRequestBuilder.append(BlobBatchHelper.HTTP_NEWLINE); @@ -140,4 +143,27 @@ int getOperationCount() { private static void appendWithNewline(StringBuilder stringBuilder, String value) { stringBuilder.append(value).append(BlobBatchHelper.HTTP_NEWLINE); } + + /* + * Rejects any carriage-return or line-feed character in an inner request header name or value before it is + * serialized into the multipart batch body. The inner headers are emitted as bytes inside the outer request body, + * so the HTTP transport layer never validates them. Without this check, a caller-controlled header value (such as a + * blob tag condition supplied via BlobRequestConditions.setTagsConditions) could terminate the intended header and + * inject additional Azure Storage operation-control headers. Rejecting (rather than stripping) preserves the + * semantics of the authorized request. + */ + private static String validateHeader(String value) { + if (value != null) { + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c == '\r' || c == '\n') { + throw LOGGER.logExceptionAsError(new IllegalArgumentException( + "Batch operation header names and values must not contain carriage-return ('\\r') or " + + "line-feed ('\\n') characters. Prohibited character 0x" + Integer.toHexString(c) + + " found at index " + i + ".")); + } + } + } + return value; + } } diff --git a/sdk/storage/azure-storage-blob-batch/src/test/java/com/azure/storage/blob/batch/BlobBatchHeaderInjectionTests.java b/sdk/storage/azure-storage-blob-batch/src/test/java/com/azure/storage/blob/batch/BlobBatchHeaderInjectionTests.java new file mode 100644 index 0000000000000..a21abebffc34d --- /dev/null +++ b/sdk/storage/azure-storage-blob-batch/src/test/java/com/azure/storage/blob/batch/BlobBatchHeaderInjectionTests.java @@ -0,0 +1,136 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.storage.blob.batch; + +import com.azure.core.http.HttpHeaderName; +import com.azure.core.http.HttpMethod; +import com.azure.core.http.HttpRequest; +import com.azure.storage.blob.models.BlobRequestConditions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Collection; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Regression tests for Blob Batch inner-header CRLF injection. + * + *
Inner request header names and values are serialized into the multipart batch body. Carriage return / line feed + * characters must be rejected on both the name and value so an attacker cannot terminate the intended header and inject + * a second Azure Storage operation-control header (such as {@code x-ms-delete-snapshots}).
+ * + *The header-value boundary is reachable through the public + * {@link BlobRequestConditions#setTagsConditions(String)} API. The header-name boundary is reachable by any + * pipeline policy that calls {@code HttpRequest#setHeader} with an arbitrary name, so it is exercised directly against + * the serializer.
+ */ +public class BlobBatchHeaderInjectionTests extends BlobBatchTestBase { + + private static final String BLOB_URL = "https://account.blob.core.windows.net/victim/target"; + + private BlobBatchClient batchClient; + + @Override + public void beforeTest() { + super.beforeTest(); + batchClient = new BlobBatchClientBuilder(primaryBlobServiceAsyncClient).buildClient(); + } + + private static String serializeBody(BlobBatch batch) { + // prepareBlobBatchSubmission builds the batch body without sending a network request, so this exercises the + // serialization path (including inner-header validation) offline. + return serializeBody(batch.prepareBlobBatchSubmission().block()); + } + + private static String serializeBody(BlobBatchOperationInfo info) { + StringBuilder sb = new StringBuilder(); + Collection