From f2ff72116c8edf011260b7009fe74a546d0ef3a3 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 12 Feb 2026 04:53:52 +0000 Subject: [PATCH] fix(s3): reject CRLF characters in S3 header value options Validate contentDisposition, type (content-type), and contentEncoding options for CR, LF, and null bytes to prevent HTTP header injection (CRLF injection) in S3 API requests. These values were previously passed through to raw HTTP headers without validation, bypassing the FetchHeaders validation layer. Co-Authored-By: Claude --- src/bun.js/webcore/Blob.zig | 6 + src/s3/credentials.zig | 28 ++- test/js/bun/s3/s3-crlf-injection.test.ts | 210 +++++++++++++++++++++++ 3 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 test/js/bun/s3/s3-crlf-injection.test.ts diff --git a/src/bun.js/webcore/Blob.zig b/src/bun.js/webcore/Blob.zig index 28056fd2e0..4ad4f43428 100644 --- a/src/bun.js/webcore/Blob.zig +++ b/src/bun.js/webcore/Blob.zig @@ -2681,6 +2681,9 @@ pub fn getWriter( return globalThis.throwInvalidArgumentType("write", "options.contentDisposition", "string"); } content_disposition_str = try content_disposition.toSlice(globalThis, bun.default_allocator); + if (!bun.S3.S3Credentials.isValidHTTPHeaderValue(content_disposition_str.?.slice())) { + return globalThis.throwInvalidArgumentType("write", "options.contentDisposition", "a valid HTTP header value (cannot contain \\r, \\n, or null bytes)"); + } } var content_encoding_str: ?ZigString.Slice = null; defer if (content_encoding_str) |ce| ce.deinit(); @@ -2689,6 +2692,9 @@ pub fn getWriter( return globalThis.throwInvalidArgumentType("write", "options.contentEncoding", "string"); } content_encoding_str = try content_encoding.toSlice(globalThis, bun.default_allocator); + if (!bun.S3.S3Credentials.isValidHTTPHeaderValue(content_encoding_str.?.slice())) { + return globalThis.throwInvalidArgumentType("write", "options.contentEncoding", "a valid HTTP header value (cannot contain \\r, \\n, or null bytes)"); + } } var credentialsWithOptions = try s3.getCredentialsWithOptions(options, globalThis); defer credentialsWithOptions.deinit(); diff --git a/src/s3/credentials.zig b/src/s3/credentials.zig index 10588c1334..0534e560c8 100644 --- a/src/s3/credentials.zig +++ b/src/s3/credentials.zig @@ -20,6 +20,16 @@ pub const S3Credentials = struct { return @sizeOf(S3Credentials) + this.accessKeyId.len + this.region.len + this.secretAccessKey.len + this.endpoint.len + this.bucket.len; } + /// Validates that a string intended for use as an HTTP header value does not + /// contain characters that could enable header injection (CRLF injection). + /// Rejects carriage return (0x0D), line feed (0x0A), and null bytes (0x00). + pub fn isValidHTTPHeaderValue(value: []const u8) bool { + for (value) |c| { + if (c == '\r' or c == '\n' or c == 0) return false; + } + return true; + } + fn hashConst(acl: []const u8) u64 { var hasher = std.hash.Wyhash.init(0); var remain = acl; @@ -221,7 +231,11 @@ pub const S3Credentials = struct { defer str.deref(); if (str.tag != .Empty and str.tag != .Dead) { new_credentials._contentDispositionSlice = str.toUTF8(bun.default_allocator); - new_credentials.content_disposition = new_credentials._contentDispositionSlice.?.slice(); + const slice = new_credentials._contentDispositionSlice.?.slice(); + if (!isValidHTTPHeaderValue(slice)) { + return globalObject.throwInvalidArgumentTypeValue("contentDisposition", "a valid HTTP header value (cannot contain \\r, \\n, or null bytes)", js_value); + } + new_credentials.content_disposition = slice; } } else { return globalObject.throwInvalidArgumentTypeValue("contentDisposition", "string", js_value); @@ -236,7 +250,11 @@ pub const S3Credentials = struct { defer str.deref(); if (str.tag != .Empty and str.tag != .Dead) { new_credentials._contentTypeSlice = str.toUTF8(bun.default_allocator); - new_credentials.content_type = new_credentials._contentTypeSlice.?.slice(); + const slice = new_credentials._contentTypeSlice.?.slice(); + if (!isValidHTTPHeaderValue(slice)) { + return globalObject.throwInvalidArgumentTypeValue("type", "a valid HTTP header value (cannot contain \\r, \\n, or null bytes)", js_value); + } + new_credentials.content_type = slice; } } else { return globalObject.throwInvalidArgumentTypeValue("type", "string", js_value); @@ -251,7 +269,11 @@ pub const S3Credentials = struct { defer str.deref(); if (str.tag != .Empty and str.tag != .Dead) { new_credentials._contentEncodingSlice = str.toUTF8(bun.default_allocator); - new_credentials.content_encoding = new_credentials._contentEncodingSlice.?.slice(); + const slice = new_credentials._contentEncodingSlice.?.slice(); + if (!isValidHTTPHeaderValue(slice)) { + return globalObject.throwInvalidArgumentTypeValue("contentEncoding", "a valid HTTP header value (cannot contain \\r, \\n, or null bytes)", js_value); + } + new_credentials.content_encoding = slice; } } else { return globalObject.throwInvalidArgumentTypeValue("contentEncoding", "string", js_value); diff --git a/test/js/bun/s3/s3-crlf-injection.test.ts b/test/js/bun/s3/s3-crlf-injection.test.ts new file mode 100644 index 0000000000..52ecab0200 --- /dev/null +++ b/test/js/bun/s3/s3-crlf-injection.test.ts @@ -0,0 +1,210 @@ +import { S3Client } from "bun"; +import { describe, expect, it } from "bun:test"; + +// Test that S3 header values reject CRLF characters to prevent HTTP header injection. +// This validates the fix for header injection via contentDisposition, type, and contentEncoding. + +describe("S3 - CRLF Header Injection Prevention", () => { + const baseOptions = { + accessKeyId: "test", + secretAccessKey: "test", + endpoint: "http://127.0.0.1:1234", + bucket: "test", + }; + + describe("contentDisposition", () => { + it("should reject CRLF", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentDisposition: "attachment\r\nX-Injected: true", + }); + }).toThrow(/contentDisposition/); + }); + + it("should reject lone CR", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentDisposition: "attachment\rX-Injected: true", + }); + }).toThrow(/contentDisposition/); + }); + + it("should reject lone LF", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentDisposition: "attachment\nX-Injected: true", + }); + }).toThrow(/contentDisposition/); + }); + + it("should reject null bytes", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentDisposition: "attachment\x00injected", + }); + }).toThrow(/contentDisposition/); + }); + + it("should allow valid values", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentDisposition: 'attachment; filename="report.pdf"', + }); + }).not.toThrow(); + }); + }); + + describe("type (content-type)", () => { + it("should reject CRLF", () => { + expect(() => { + new S3Client({ + ...baseOptions, + type: "text/plain\r\nX-Injected: true", + }); + }).toThrow(/type/); + }); + + it("should reject lone LF", () => { + expect(() => { + new S3Client({ + ...baseOptions, + type: "text/plain\nX-Injected: true", + }); + }).toThrow(/type/); + }); + + it("should allow valid values", () => { + expect(() => { + new S3Client({ + ...baseOptions, + type: "application/octet-stream", + }); + }).not.toThrow(); + }); + }); + + describe("contentEncoding", () => { + it("should reject CRLF", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentEncoding: "gzip\r\nX-Injected: true", + }); + }).toThrow(/contentEncoding/); + }); + + it("should reject lone LF", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentEncoding: "gzip\nX-Injected: true", + }); + }).toThrow(/contentEncoding/); + }); + + it("should allow valid values", () => { + expect(() => { + new S3Client({ + ...baseOptions, + contentEncoding: "gzip", + }); + }).not.toThrow(); + }); + }); + + describe("per-file options", () => { + it("should reject CRLF in contentDisposition on file()", () => { + const client = new S3Client(baseOptions); + expect(() => { + client.file("test-key", { + contentDisposition: "attachment\r\nX-Injected: true", + }); + }).toThrow(/contentDisposition/); + }); + + it("should reject CRLF in type on file()", () => { + const client = new S3Client(baseOptions); + expect(() => { + client.file("test-key", { + type: "text/plain\r\nX-Injected: true", + }); + }).toThrow(/type/); + }); + + it("should reject CRLF in contentEncoding on file()", () => { + const client = new S3Client(baseOptions); + expect(() => { + client.file("test-key", { + contentEncoding: "gzip\r\nX-Injected: true", + }); + }).toThrow(/contentEncoding/); + }); + }); + + describe("write() options path", () => { + it("should reject CRLF in contentDisposition on write()", async () => { + const client = new S3Client(baseOptions); + const file = client.file("test-key"); + expect(file.write("data", { contentDisposition: "attachment\r\nX-Injected: true" })).rejects.toThrow( + /contentDisposition/, + ); + }); + + it("should reject CRLF in contentEncoding on write()", async () => { + const client = new S3Client(baseOptions); + const file = client.file("test-key"); + expect(file.write("data", { contentEncoding: "gzip\r\nX-Injected: true" })).rejects.toThrow(/contentEncoding/); + }); + }); + + it("CRLF in contentDisposition should not reach the wire", async () => { + const requests: string[] = []; + const server = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + data(socket, data) { + requests.push(data.toString()); + socket.write("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + socket.end(); + }, + open() {}, + close() {}, + error() {}, + }, + }); + + try { + const client = new S3Client({ + accessKeyId: "test", + secretAccessKey: "testsecret1234567890123456789012", + endpoint: `http://127.0.0.1:${server.port}`, + bucket: "test", + }); + + const file = client.file("test-key"); + try { + await file.write("test-data", { + contentDisposition: "attachment; filename=report.pdf\r\nX-Injected: true", + }); + } catch { + // Expected to throw + } + + // Give a moment for any request to arrive + await Bun.sleep(200); + + // Verify no injected header made it to the wire + for (const req of requests) { + expect(req).not.toContain("X-Injected"); + } + } finally { + server.stop(); + } + }); +});