mirror of
https://github.com/oven-sh/bun
synced 2026-02-18 14:51:52 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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);
|
||||
|
||||
210
test/js/bun/s3/s3-crlf-injection.test.ts
Normal file
210
test/js/bun/s3/s3-crlf-injection.test.ts
Normal file
@@ -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();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user