From 9a2dfee3caa5ef616653929eab3ad112d380f752 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 28 Jul 2025 00:13:17 -0700 Subject: [PATCH] Fix env loader buffer overflow by using stack fallback allocator (#21416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixed buffer overflow in env_loader when parsing large environment variables with escape sequences - Replaced fixed 4096-byte buffer with a stack fallback allocator that automatically switches to heap allocation for larger values - Added comprehensive tests to prevent regression ## Background The env_loader previously used a fixed threadlocal buffer that could overflow when parsing environment variables containing escape sequences. This caused crashes when the parsed value exceeded 4KB. ## Changes - Replaced fixed buffer with `StackFallbackAllocator` that uses 4KB stack buffer for common cases and falls back to heap for larger values - Updated all env parsing functions to accept a reusable buffer parameter - Added proper memory cleanup with defer statements ## Test plan - [x] Added test cases for large environment variables with escape sequences - [x] Added test for values larger than 4KB - [x] Added edge case tests (empty quotes, escape at EOF) - [x] All existing env tests continue to pass fixes #11627 fixes BAPI-1274 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Claude Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/node/node_util_binding.zig | 2 +- src/cli/create_command.zig | 6 +- src/cli/run_command.zig | 4 +- src/cli/upgrade_command.zig | 2 +- src/env_loader.zig | 168 +++++++++++++------------- src/install/PackageManager.zig | 2 +- src/install/lockfile.zig | 2 +- src/transpiler.zig | 4 +- test/cli/run/env.test.ts | 46 ++++++- 9 files changed, 139 insertions(+), 97 deletions(-) diff --git a/src/bun.js/node/node_util_binding.zig b/src/bun.js/node/node_util_binding.zig index bbff581841..89fd03cde0 100644 --- a/src/bun.js/node/node_util_binding.zig +++ b/src/bun.js/node/node_util_binding.zig @@ -216,7 +216,7 @@ pub fn parseEnv(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun. var map = envloader.Map.init(allocator); var p = envloader.Loader.init(&map, allocator); - p.loadFromString(str.slice(), true, false); + try p.loadFromString(str.slice(), true, false); var obj = jsc.JSValue.createEmptyObject(globalThis, map.map.count()); for (map.map.keys(), map.map.values()) |k, v| { diff --git a/src/cli/create_command.zig b/src/cli/create_command.zig index 4f7ed16287..e90ec614bd 100644 --- a/src/cli/create_command.zig +++ b/src/cli/create_command.zig @@ -213,7 +213,7 @@ pub const CreateCommand = struct { break :brk DotEnv.Loader.init(map, ctx.allocator); }; - env_loader.loadProcess(); + try env_loader.loadProcess(); const dirname: string = brk: { if (positionals.len == 1) { @@ -1683,7 +1683,7 @@ pub const CreateCommand = struct { break :brk DotEnv.Loader.init(map, ctx.allocator); }; - env_loader.loadProcess(); + try env_loader.loadProcess(); // var unsupported_packages = UnsupportedPackages{}; const template = brk: { @@ -2282,7 +2282,7 @@ pub const CreateListExamplesCommand = struct { break :brk DotEnv.Loader.init(map, ctx.allocator); }; - env_loader.loadProcess(); + try env_loader.loadProcess(); var progress = Progress{}; progress.supports_ansi_escape_codes = Output.enable_ansi_colors_stderr; diff --git a/src/cli/run_command.zig b/src/cli/run_command.zig index 681e8e286c..800d3a87cb 100644 --- a/src/cli/run_command.zig +++ b/src/cli/run_command.zig @@ -786,7 +786,7 @@ pub const RunCommand = struct { this_transpiler.resolver.store_fd = false; if (env == null) { - this_transpiler.env.loadProcess(); + try this_transpiler.env.loadProcess(); if (this_transpiler.env.get("NODE_ENV")) |node_env| { if (strings.eqlComptime(node_env, "production")) { @@ -973,7 +973,7 @@ pub const RunCommand = struct { const root_dir_info = (this_transpiler.resolver.readDirInfo(this_transpiler.fs.top_level_dir) catch null) orelse return shell_out; { - this_transpiler.env.loadProcess(); + try this_transpiler.env.loadProcess(); if (this_transpiler.env.get("NODE_ENV")) |node_env| { if (strings.eqlComptime(node_env, "production")) { diff --git a/src/cli/upgrade_command.zig b/src/cli/upgrade_command.zig index cc83cbc352..08006f4c8c 100644 --- a/src/cli/upgrade_command.zig +++ b/src/cli/upgrade_command.zig @@ -358,7 +358,7 @@ pub const UpgradeCommand = struct { break :brk DotEnv.Loader.init(map, ctx.allocator); }; - env_loader.loadProcess(); + try env_loader.loadProcess(); const use_canary = brk: { const default_use_canary = Environment.is_canary; diff --git a/src/env_loader.zig b/src/env_loader.zig index d2399ff509..fb1787f758 100644 --- a/src/env_loader.zig +++ b/src/env_loader.zig @@ -393,7 +393,7 @@ pub const Loader = struct { const value: string = entry.value_ptr.value; if (strings.startsWith(entry.key_ptr.*, prefix)) { - const key_str = std.fmt.allocPrint(key_allocator, "process.env.{s}", .{entry.key_ptr.*}) catch unreachable; + const key_str = try std.fmt.allocPrint(key_allocator, "process.env.{s}", .{entry.key_ptr.*}); e_strings[0] = js_ast.E.String{ .data = if (value.len > 0) @@ -442,7 +442,7 @@ pub const Loader = struct { } else { while (iter.next()) |entry| { const value: string = entry.value_ptr.value; - const key = std.fmt.allocPrint(key_allocator, "process.env.{s}", .{entry.key_ptr.*}) catch unreachable; + const key = try std.fmt.allocPrint(key_allocator, "process.env.{s}", .{entry.key_ptr.*}); e_strings[0] = js_ast.E.String{ .data = if (entry.value_ptr.value.len > 0) @@ -484,21 +484,21 @@ pub const Loader = struct { }; } - pub fn loadProcess(this: *Loader) void { + pub fn loadProcess(this: *Loader) OOM!void { if (this.did_load_process) return; - this.map.map.ensureTotalCapacity(std.os.environ.len) catch unreachable; + try this.map.map.ensureTotalCapacity(std.os.environ.len); for (std.os.environ) |_env| { var env = bun.span(_env); if (strings.indexOfChar(env, '=')) |i| { const key = env[0..i]; const value = env[i + 1 ..]; if (key.len > 0) { - this.map.put(key, value) catch unreachable; + try this.map.put(key, value); } } else { if (env.len > 0) { - this.map.put(env, "") catch unreachable; + try this.map.put(env, ""); } } } @@ -506,9 +506,11 @@ pub const Loader = struct { } // mostly for tests - pub fn loadFromString(this: *Loader, str: string, comptime overwrite: bool, comptime expand: bool) void { + pub fn loadFromString(this: *Loader, str: string, comptime overwrite: bool, comptime expand: bool) OOM!void { const source = &logger.Source.initPathString("test", str); - Parser.parse(source, this.allocator, this.map, overwrite, false, expand); + var value_buffer = std.ArrayList(u8).init(this.allocator); + defer value_buffer.deinit(); + try Parser.parse(source, this.allocator, this.map, &value_buffer, overwrite, false, expand); std.mem.doNotOptimizeAway(&source); } @@ -521,8 +523,13 @@ pub const Loader = struct { ) !void { const start = std.time.nanoTimestamp(); + // Create a reusable buffer with stack fallback for parsing multiple files + var stack_fallback = std.heap.stackFallback(4096, this.allocator); + var value_buffer = std.ArrayList(u8).init(stack_fallback.get()); + defer value_buffer.deinit(); + if (env_files.len > 0) { - try this.loadExplicitFiles(env_files); + try this.loadExplicitFiles(env_files, &value_buffer); } else { // Do not automatically load .env files in `bun run