[Storage] Blob Batch Inner-Header CRLF Injection Bugfix

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
 

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"
 }

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;
+    }
 }

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.
+ *
+ * <p>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}).</p>
+ *
+ * <p>The header-<em>value</em> boundary is reachable through the public
+ * {@link BlobRequestConditions#setTagsConditions(String)} API. The header-<em>name</em> boundary is reachable by any
+ * pipeline policy that calls {@code HttpRequest#setHeader} with an arbitrary name, so it is exercised directly against
+ * the serializer.</p>
+ */
+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<ByteBuffer> 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();
+    }
+
+    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 = {
+            "\"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);
+
+        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<Void>(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<Void>(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");
+    }
+}