From a721934539c462e849c4de8597d349aeffa97bdb Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 23 Apr 2026 18:16:50 +0000 Subject: [PATCH] fix(s3): double deref of path string on error in S3Client static methods When S3Client.presign/unlink/write/size/exists/stat are called with a path and the underlying operation throws (e.g. missing credentials), the path was freed twice: once by blob.deinit() via the blob store, and again by the errdefer on path_or_blob. Clear path_or_blob after ownership is transferred to the blob so the errdefer is a no-op. --- src/bun.js/webcore/S3File.zig | 6 ++++++ test/js/bun/s3/s3.test.ts | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/bun.js/webcore/S3File.zig b/src/bun.js/webcore/S3File.zig index 41e900fb987f..18530a834fe7 100644 --- a/src/bun.js/webcore/S3File.zig +++ b/src/bun.js/webcore/S3File.zig @@ -84,6 +84,7 @@ pub fn presign(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.J } const options = args.nextEat(); var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return try getPresignUrlFrom(&blob, globalThis, options); }, @@ -114,6 +115,7 @@ pub fn unlink(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JS } const options = args.nextEat(); var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return try blob.store.?.data.s3.unlink(blob.store.?, globalThis, options); }, @@ -151,6 +153,7 @@ pub fn write(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSE return globalThis.throwInvalidArguments("Expected a S3 or path to upload", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); var blob_internal: PathOrBlob = .{ .blob = blob }; @@ -190,6 +193,7 @@ pub fn size(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSEr return globalThis.throwInvalidArguments("Expected a S3 or path to get size", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return S3BlobStatTask.size(globalThis, &blob); @@ -223,6 +227,7 @@ pub fn exists(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JS return globalThis.throwInvalidArguments("Expected a S3 or path to check if it exists", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return S3BlobStatTask.exists(globalThis, &blob); @@ -574,6 +579,7 @@ pub fn stat(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSEr return globalThis.throwInvalidArguments("Expected a S3 or path to get size", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return S3BlobStatTask.stat(globalThis, &blob); diff --git a/test/js/bun/s3/s3.test.ts b/test/js/bun/s3/s3.test.ts index 6ede69dddc6c..598e8ec132a6 100644 --- a/test/js/bun/s3/s3.test.ts +++ b/test/js/bun/s3/s3.test.ts @@ -1564,6 +1564,10 @@ describe.concurrent("s3 missing credentials", () => { await Bun.s3.presign("test"); }); }); + it("S3Client.presign static", () => { + expect(() => S3Client.presign("some/bucket/key")).toThrow("Missing S3 credentials"); + expect(() => S3Client.presign("some/bucket/key", {})).toThrow("Missing S3 credentials"); + }); it("file", async () => { assertMissingCredentials(async () => { await Bun.s3.file("test").text();