Skip to content

Commit

Permalink
fix double free with invalid TLSOptions (#14648)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dylan-conway
dylan-conway authored Oct 18, 2024
1 parent b652136 commit f3b658d
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 51 deletions.
11 changes: 6 additions & 5 deletions src/bun.js/api/BunObject.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3305,21 +3305,22 @@ pub fn serve(
const exception = &exception_;

var args = JSC.Node.ArgumentsSlice.init(globalObject.bunVM(), arguments);
var config_ = JSC.API.ServerConfig.fromJS(globalObject.ptr(), &args, exception);
var config: JSC.API.ServerConfig = .{};
JSC.API.ServerConfig.fromJS(globalObject, &config, &args, exception);
if (exception[0] != null) {
config_.deinit();
config.deinit();

globalObject.throwValue(exception_[0].?.value());
return .undefined;
return .zero;
}

if (globalObject.hasException()) {
config_.deinit();
config.deinit();

return .zero;
}

break :brk config_;
break :brk config;
};

var exception_value: *JSC.JSValue = undefined;
Expand Down
99 changes: 53 additions & 46 deletions src/bun.js/api/server.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1174,11 +1174,16 @@ pub const ServerConfig = struct {
}
};

pub fn fromJS(global: *JSC.JSGlobalObject, arguments: *JSC.Node.ArgumentsSlice, exception: JSC.C.ExceptionRef) ServerConfig {
pub fn fromJS(
global: *JSC.JSGlobalObject,
args: *ServerConfig,
arguments: *JSC.Node.ArgumentsSlice,
exception: JSC.C.ExceptionRef,
) void {
const vm = arguments.vm;
const env = vm.bundler.env;

var args = ServerConfig{
args.* = .{
.address = .{
.tcp = .{
.port = 3000,
Expand Down Expand Up @@ -1236,13 +1241,13 @@ pub const ServerConfig = struct {
if (arguments.next()) |arg| {
if (!arg.isObject()) {
JSC.throwInvalidArguments("Bun.serve expects an object", .{}, global, exception);
return args;
return;
}

if (arg.get(global, "static")) |static| {
if (!static.isObject()) {
JSC.throwInvalidArguments("Bun.serve expects 'static' to be an object shaped like { [pathname: string]: Response }", .{}, global, exception);
return args;
return;
}

var iter = JSC.JSPropertyIterator(.{
Expand All @@ -1259,13 +1264,13 @@ pub const ServerConfig = struct {
if (path.len == 0 or path[0] != '/') {
bun.default_allocator.free(path);
JSC.throwInvalidArguments("Invalid static route \"{s}\". path must start with '/'", .{path}, global, exception);
return args;
return;
}

if (!is_ascii) {
bun.default_allocator.free(path);
JSC.throwInvalidArguments("Invalid static route \"{s}\". Please encode all non-ASCII characters in the path.", .{path}, global, exception);
return args;
return;
}

if (StaticRoute.fromJS(global, value)) |route| {
Expand All @@ -1275,28 +1280,28 @@ pub const ServerConfig = struct {
}) catch bun.outOfMemory();
} else if (global.hasException()) {
bun.default_allocator.free(path);
return args;
return;
} else {
Output.panic("Internal error: expected exception or static route", .{});
}
}
}

if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.get(global, "idleTimeout")) |value| {
if (!value.isUndefinedOrNull()) {
if (!value.isAnyInt()) {
JSC.throwInvalidArguments("Bun.serve expects idleTimeout to be an integer", .{}, global, exception);

return args;
return;
}
args.has_idleTimeout = true;

const idleTimeout: u64 = @intCast(@max(value.toInt64(), 0));
if (idleTimeout > 255) {
JSC.throwInvalidArguments("Bun.serve expects idleTimeout to be 255 or less", .{}, global, exception);
return args;
return;
}

args.idleTimeout = @truncate(idleTimeout);
Expand All @@ -1309,7 +1314,7 @@ pub const ServerConfig = struct {
if (args.ssl_config) |*conf| {
conf.deinit();
}
return args;
return;
}

if (WebSocketServer.onCreate(global, websocket_object)) |wss| {
Expand All @@ -1318,10 +1323,10 @@ pub const ServerConfig = struct {
if (args.ssl_config) |*conf| {
conf.deinit();
}
return args;
return;
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.getTruthy(global, "port")) |port_| {
args.address.tcp.port = @as(
Expand All @@ -1333,7 +1338,7 @@ pub const ServerConfig = struct {
);
port = args.address.tcp.port;
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.getTruthy(global, "baseURI")) |baseURI| {
var sliced = baseURI.toSlice(global, bun.default_allocator);
Expand All @@ -1343,7 +1348,7 @@ pub const ServerConfig = struct {
args.base_uri = bun.default_allocator.dupe(u8, sliced.slice()) catch unreachable;
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.getTruthy(global, "hostname") orelse arg.getTruthy(global, "host")) |host| {
const host_str = host.toSlice(
Expand All @@ -1357,7 +1362,7 @@ pub const ServerConfig = struct {
has_hostname = true;
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.getTruthy(global, "unix")) |unix| {
const unix_str = unix.toSlice(
Expand All @@ -1368,13 +1373,13 @@ pub const ServerConfig = struct {
if (unix_str.len > 0) {
if (has_hostname) {
JSC.throwInvalidArguments("Cannot specify both hostname and unix", .{}, global, exception);
return args;
return;
}

args.address = .{ .unix = bun.default_allocator.dupeZ(u8, unix_str.slice()) catch unreachable };
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.get(global, "id")) |id| {
if (id.isUndefinedOrNull()) {
Expand All @@ -1392,67 +1397,67 @@ pub const ServerConfig = struct {
}
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.get(global, "development")) |dev| {
args.development = dev.coerce(bool, global);
args.reuse_port = !args.development;
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.get(global, "reusePort")) |dev| {
args.reuse_port = dev.coerce(bool, global);
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.get(global, "inspector")) |inspector| {
args.inspector = inspector.coerce(bool, global);

if (args.inspector and !args.development) {
JSC.throwInvalidArguments("Cannot enable inspector in production. Please set development: true in Bun.serve()", .{}, global, exception);
return args;
return;
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.getTruthy(global, "maxRequestBodySize")) |max_request_body_size| {
if (max_request_body_size.isNumber()) {
args.max_request_body_size = @as(u64, @intCast(@max(0, max_request_body_size.toInt64())));
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.getTruthyComptime(global, "error")) |onError| {
if (!onError.isCallable(global.vm())) {
JSC.throwInvalidArguments("Expected error to be a function", .{}, global, exception);
return args;
return;
}
const onErrorSnapshot = onError.withAsyncContextIfNeeded(global);
args.onError = onErrorSnapshot;
onErrorSnapshot.protect();
}
if (global.hasException()) return args;
if (global.hasException()) return;

if (arg.getTruthy(global, "fetch")) |onRequest_| {
if (!onRequest_.isCallable(global.vm())) {
JSC.throwInvalidArguments("Expected fetch() to be a function", .{}, global, exception);
return args;
return;
}
const onRequest = onRequest_.withAsyncContextIfNeeded(global);
JSC.C.JSValueProtect(global, onRequest.asObjectRef());
args.onRequest = onRequest;
} else {
if (global.hasException()) return args;
if (global.hasException()) return;
JSC.throwInvalidArguments("Expected fetch() to be a function", .{}, global, exception);
return args;
return;
}

if (arg.getTruthy(global, "tls")) |tls| {
if (tls.jsType().isArray()) {
var value_iter = tls.arrayIterator(global);
if (value_iter.len == 1) {
JSC.throwInvalidArguments("tls option expects at least 1 tls object", .{}, global, exception);
return args;
return;
}
while (value_iter.next()) |item| {
if (SSLConfig.inJS(vm, global, item, exception)) |ssl_config| {
Expand All @@ -1463,7 +1468,7 @@ pub const ServerConfig = struct {
var config = ssl_config;
defer config.deinit();
JSC.throwInvalidArguments("SNI tls object must have a serverName", .{}, global, exception);
return args;
return;
}
if (args.sni == null) {
args.sni = bun.BabyList(SSLConfig).initCapacity(bun.default_allocator, value_iter.len - 1) catch bun.outOfMemory();
Expand All @@ -1474,11 +1479,11 @@ pub const ServerConfig = struct {
}

if (exception.* != null) {
return args;
return;
}

if (global.hasException()) {
return args;
return;
}
}
} else {
Expand All @@ -1487,15 +1492,15 @@ pub const ServerConfig = struct {
}

if (exception.* != null) {
return args;
return;
}

if (global.hasException()) {
return args;
return;
}
}
}
if (global.hasException()) return args;
if (global.hasException()) return;

// @compatibility Bun v0.x - v0.2.1
// this used to be top-level, now it's "tls" object
Expand All @@ -1505,16 +1510,16 @@ pub const ServerConfig = struct {
}

if (exception.* != null) {
return args;
return;
}

if (global.hasException()) {
return args;
return;
}
}
} else {
JSC.throwInvalidArguments("Bun.serve expects an object", .{}, global, exception);
return args;
return;
}

if (args.base_uri.len > 0) {
Expand All @@ -1523,14 +1528,14 @@ pub const ServerConfig = struct {
JSC.throwInvalidArguments("baseURI must have a hostname", .{}, global, exception);
bun.default_allocator.free(@constCast(args.base_uri));
args.base_uri = "";
return args;
return;
}

if (!strings.isAllASCII(args.base_uri)) {
JSC.throwInvalidArguments("Unicode baseURI must already be encoded for now.\nnew URL(baseuRI).toString() should do the trick.", .{}, global, exception);
bun.default_allocator.free(@constCast(args.base_uri));
args.base_uri = "";
return args;
return;
}

if (args.base_url.protocol.len == 0) {
Expand Down Expand Up @@ -1598,7 +1603,7 @@ pub const ServerConfig = struct {
JSC.throwInvalidArguments("Unicode hostnames must already be encoded for now.\nnew URL(input).hostname should do the trick.", .{}, global, exception);
bun.default_allocator.free(@constCast(args.base_uri));
args.base_uri = "";
return args;
return;
}

args.base_url = URL.parse(args.base_uri);
Expand All @@ -1610,17 +1615,17 @@ pub const ServerConfig = struct {
JSC.throwInvalidArguments("baseURI must have a hostname", .{}, global, exception);
bun.default_allocator.free(@constCast(args.base_uri));
args.base_uri = "";
return args;
return;
}

if (args.base_url.username.len > 0 or args.base_url.password.len > 0) {
JSC.throwInvalidArguments("baseURI can't have a username or password", .{}, global, exception);
bun.default_allocator.free(@constCast(args.base_uri));
args.base_uri = "";
return args;
return;
}

return args;
return;
}
};

Expand Down Expand Up @@ -6167,7 +6172,9 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
defer args_slice.deinit();
var exception_ref = [_]JSC.C.JSValueRef{null};
const exception: JSC.C.ExceptionRef = &exception_ref;
var new_config = ServerConfig.fromJS(globalThis, &args_slice, exception);

var new_config: ServerConfig = .{};
ServerConfig.fromJS(globalThis, &new_config, &args_slice, exception);
if (exception.* != null) {
new_config.deinit();
globalThis.throwValue(exception_ref[0].?.value());
Expand Down
Loading

0 comments on commit f3b658d

Please sign in to comment.