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