From 004ae4b66eb0e718ed6f723d91a909c9a990d98a Mon Sep 17 00:00:00 2001 From: Isabelle Date: Thu, 10 Sep 2026 11:16:22 -0700 Subject: [PATCH 1/3] fix, tests, and changelog --- .../azure-storage-blob-batch/CHANGELOG.md | 4 + .../blob/batch/BlobBatchOperationInfo.java | 28 +++++- .../batch/BlobBatchHeaderInjectionTests.java | 86 +++++++++++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 sdk/storage/azure-storage-blob-batch/src/test/java/com/azure/storage/blob/batch/BlobBatchHeaderInjectionTests.java 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/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..43546e545e5b9 --- /dev/null +++ b/sdk/storage/azure-storage-blob-batch/src/test/java/com/azure/storage/blob/batch/BlobBatchHeaderInjectionTests.java @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.storage.blob.batch; + +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. + * + *

A caller-controlled {@code x-ms-if-tags} condition supplied through the public + * {@link BlobRequestConditions#setTagsConditions(String)} API is serialized into the multipart batch body. Carriage + * return / line feed characters must be rejected so an attacker cannot terminate the intended header and inject a + * second Azure Storage operation-control header (such as {@code x-ms-delete-snapshots}).

+ */ +public class BlobBatchHeaderInjectionTests extends BlobBatchTestBase { + + 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. + BlobBatchOperationInfo info = batch.prepareBlobBatchSubmission().block(); + StringBuilder sb = new StringBuilder(); + Collection body = info.getBody(); + for (ByteBuffer buffer : body) { + byte[] bytes = new byte[buffer.remaining()]; + buffer.duplicate().get(bytes); + sb.append(new String(bytes, StandardCharsets.UTF_8)); + } + return sb.toString(); + } + + @ParameterizedTest + @ValueSource( + strings = { + "\"owner\" = 'attacker'\r\nx-ms-delete-snapshots: include", // CRLF + "\"owner\" = 'attacker'\rx-ms-delete-snapshots: include", // lone CR + "\"owner\" = 'attacker'\nx-ms-delete-snapshots: include" // lone LF + }) + public void lineBreakInTagsConditionIsRejected(String maliciousCondition) { + BlobBatch batch = batchClient.getBlobBatch(); + // deleteOptions is intentionally null - the application did NOT authorize snapshot deletion. + batch.deleteBlob("victim", "target", null, new BlobRequestConditions().setTagsConditions(maliciousCondition)); + + // The line-break-bearing value must be rejected before it is serialized into the batch body. + assertThrows(IllegalArgumentException.class, () -> batch.prepareBlobBatchSubmission().block()); + } + + @Test + public void cleanTagsConditionIsPreserved() { + BlobBatch batch = batchClient.getBlobBatch(); + batch.deleteBlob("victim", "target", null, + new BlobRequestConditions().setTagsConditions("\"owner\" = 'owner'")); + + String body = serializeBody(batch); + + long injectedDeleteHeaderCount = 0; + for (String line : body.split("\r\n")) { + if ("x-ms-delete-snapshots: include".equalsIgnoreCase(line)) { + injectedDeleteHeaderCount++; + } + } + + assertEquals(0, injectedDeleteHeaderCount); + assertTrue(body.contains("x-ms-if-tags: \"owner\" = 'owner'"), + "Clean tag condition header should be preserved intact"); + } +} From ec13e351a699eda2d624b9310c3b885fdb7d979f Mon Sep 17 00:00:00 2001 From: Isabelle Date: Thu, 10 Sep 2026 11:32:32 -0700 Subject: [PATCH 2/3] expanding unit test coverage --- .../batch/BlobBatchHeaderInjectionTests.java | 76 +++++++++++++++---- 1 file changed, 63 insertions(+), 13 deletions(-) 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 index 43546e545e5b9..a21abebffc34d 100644 --- 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 @@ -3,6 +3,9 @@ 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; @@ -19,13 +22,19 @@ /** * Regression tests for Blob Batch inner-header CRLF injection. * - *

A caller-controlled {@code x-ms-if-tags} condition supplied through the public - * {@link BlobRequestConditions#setTagsConditions(String)} API is serialized into the multipart batch body. Carriage - * return / line feed characters must be rejected so an attacker cannot terminate the intended header and inject a - * second Azure Storage operation-control header (such as {@code x-ms-delete-snapshots}).

+ *

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 @@ -37,7 +46,10 @@ public void beforeTest() { 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. - BlobBatchOperationInfo info = batch.prepareBlobBatchSubmission().block(); + return serializeBody(batch.prepareBlobBatchSubmission().block()); + } + + private static String serializeBody(BlobBatchOperationInfo info) { StringBuilder sb = new StringBuilder(); Collection body = info.getBody(); for (ByteBuffer buffer : body) { @@ -48,6 +60,18 @@ private static String serializeBody(BlobBatch batch) { return sb.toString(); } + private static int countInjectedDeleteHeaders(String body) { + int count = 0; + for (String line : body.split("\r\n")) { + if ("x-ms-delete-snapshots: include".equalsIgnoreCase(line)) { + count++; + } + } + return count; + } + + // --- Header value boundary (reachable via the public setTagsConditions API) --- + @ParameterizedTest @ValueSource( strings = { @@ -72,15 +96,41 @@ public void cleanTagsConditionIsPreserved() { String body = serializeBody(batch); - long injectedDeleteHeaderCount = 0; - for (String line : body.split("\r\n")) { - if ("x-ms-delete-snapshots: include".equalsIgnoreCase(line)) { - injectedDeleteHeaderCount++; - } - } - - assertEquals(0, injectedDeleteHeaderCount); + assertEquals(0, countInjectedDeleteHeaders(body)); assertTrue(body.contains("x-ms-if-tags: \"owner\" = 'owner'"), "Clean tag condition header should be preserved intact"); } + + // --- Header name boundary (reachable by any policy that sets an arbitrary header name) --- + + @ParameterizedTest + @ValueSource( + strings = { + "x-ms-inject\r\nx-ms-delete-snapshots", // CRLF + "x-ms-inject\rx-ms-delete-snapshots", // lone CR + "x-ms-inject\nx-ms-delete-snapshots" // lone LF + }) + public void lineBreakInHeaderNameIsRejected(String maliciousHeaderName) { + HttpRequest request = new HttpRequest(HttpMethod.DELETE, BLOB_URL); + request.setHeader(HttpHeaderName.fromString(maliciousHeaderName), "include"); + + BlobBatchOperationInfo info = new BlobBatchOperationInfo(); + assertThrows(IllegalArgumentException.class, + () -> info.addBatchOperation(new BlobBatchOperationResponse(202), request)); + } + + @Test + public void cleanHeaderNameIsPreserved() { + HttpRequest request = new HttpRequest(HttpMethod.DELETE, BLOB_URL); + request.setHeader(HttpHeaderName.fromString("x-ms-clean-header"), "clean-value"); + + BlobBatchOperationInfo info = new BlobBatchOperationInfo(); + info.addBatchOperation(new BlobBatchOperationResponse(202), request); + + String body = serializeBody(info); + + assertEquals(0, countInjectedDeleteHeaders(body)); + assertTrue(body.contains("x-ms-clean-header: clean-value"), + "Clean header name and value should be preserved intact"); + } } From db0b0f1da96a7f04f0950c6eb7d692cb232c5174 Mon Sep 17 00:00:00 2001 From: Isabelle Date: Thu, 10 Sep 2026 13:54:56 -0700 Subject: [PATCH 3/3] adding recordings --- sdk/storage/azure-storage-blob-batch/assets.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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" }