From 514efbf73964639c5252980f4569269ee4419e09 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 14 May 2026 02:45:45 -0700 Subject: [PATCH] Input validation and bounds-checking hardening Tightens validation across the runtime, package manager, SQL drivers, HTTP/2 server, shell, bundler, and JSC bindings. --- packages/bun-uws/src/HttpParser.h | 5 ++ packages/bun-uws/src/HttpResponse.h | 6 +- src/base64/lib.rs | 4 +- .../linker_context/postProcessCSSChunk.rs | 22 +++++- .../linker_context/postProcessJSChunk.rs | 22 +++++- src/bundler/options.rs | 19 +++-- src/css/css_parser.rs | 18 ++++- src/http_jsc/websocket_client.rs | 23 +++--- src/install/bin.rs | 32 +++++++++ src/js/bun/sql.ts | 7 +- src/js/internal/debugger.ts | 4 +- src/js/internal/sql/mysql.ts | 10 ++- src/js/internal/sql/postgres.ts | 20 +++++- src/js/internal/sql/sqlite.ts | 9 +++ src/js/node/child_process.ts | 27 +++++++ src/jsc/bindings/NodeHTTP.cpp | 5 +- src/jsc/bindings/PathInlines.h | 4 +- .../node/crypto/JSCipherConstructor.cpp | 4 ++ src/jsc/bindings/node/http/NodeHTTPParser.cpp | 7 +- src/jsc/bindings/sqlite/JSSQLStatement.cpp | 58 +++++++++++++-- src/jsc/bindings/webcore/JSDOMFormData.cpp | 9 ++- .../webcore/SerializedScriptValue.cpp | 8 ++- src/libarchive/lib.rs | 38 ++++++++++ src/parsers/yaml.rs | 23 +++++- src/patch/lib.rs | 11 +++ src/resolver/package_json.rs | 47 +++++++++++++ src/runtime/api/bun/h2_frame_parser.rs | 70 ++++++++++++++++--- src/runtime/bake/DevServer/HmrSocket.rs | 25 ++++++- src/runtime/cli/pm_view_command.rs | 55 ++++++++++----- src/runtime/cli/upgrade_command.rs | 39 +++++++---- src/runtime/node/net/BlockList.rs | 29 +++++++- src/runtime/server/RequestContext.rs | 33 ++++++--- src/runtime/shell/states/Expansion.rs | 43 ++++++++++-- src/runtime/webcore/Blob.rs | 27 ++++++- src/semver/lib.rs | 21 ++---- src/shell_parser/parse.rs | 9 ++- src/sql_jsc/mysql/MySQLConnection.rs | 16 ++++- src/sql_jsc/postgres/PostgresRequest.rs | 7 +- src/sql_jsc/postgres/SASL.rs | 10 +++ src/url/lib.rs | 11 +-- 40 files changed, 714 insertions(+), 123 deletions(-) diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index b34953926df..99590a4892c 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -750,6 +750,11 @@ namespace uWS /* Error: invalid chars in field name */ return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_HEADER_TOKEN); } + /* Reject zero-length header names (RFC 9110 §5.6.3): an empty key is + * indistinguishable from the headers-array terminator and would truncate iteration. */ + if (postPaddedBuffer == preliminaryKey) [[unlikely]] { + return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_HEADER_TOKEN); + } postPaddedBuffer++; preliminaryValue = postPaddedBuffer; diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index e969e029836..721e34fcd26 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -257,7 +257,11 @@ struct HttpResponse : public AsyncSocket { /* Note: OpenSSL can be used here to speed this up somewhat */ char secWebSocketAccept[29] = {}; - WebSocketHandshake::generate(secWebSocketKey.data(), secWebSocketAccept); + /* WebSocketHandshake::generate reads exactly 24 bytes; the client-controlled + * header may be shorter, so copy into a zero-padded fixed buffer first. */ + char secWebSocketKeyBuf[24] = {}; + secWebSocketKey.copy(secWebSocketKeyBuf, sizeof(secWebSocketKeyBuf)); + WebSocketHandshake::generate(secWebSocketKeyBuf, secWebSocketAccept); writeStatus("101 Switching Protocols") ->writeHeader("Upgrade", "websocket") diff --git a/src/base64/lib.rs b/src/base64/lib.rs index 1c4376de73a..967730a476d 100644 --- a/src/base64/lib.rs +++ b/src/base64/lib.rs @@ -325,7 +325,9 @@ pub mod vlq { let encoded_ = &encoded[start..][0..(encoded.len() - start).min(VLQ_MAX_IN_BYTES + 1)]; // inlining helps for the 1 or 2 byte case, hurts a little for larger - for i in 0..(VLQ_MAX_IN_BYTES + 1) { + // Bound by the clamped slice length, not VLQ_MAX_IN_BYTES + 1, to avoid + // OOB reads on truncated/attacker-controlled mappings. + for i in 0..encoded_.len() { if ASSERT_VALID { debug_assert!(encoded_[i] < U7_MAX); // invalid base64 character } diff --git a/src/bundler/linker_context/postProcessCSSChunk.rs b/src/bundler/linker_context/postProcessCSSChunk.rs index e4dd36f59d6..369a7d43489 100644 --- a/src/bundler/linker_context/postProcessCSSChunk.rs +++ b/src/bundler/linker_context/postProcessCSSChunk.rs @@ -74,8 +74,26 @@ pub fn post_process_css_chunk( j.push_static(b"/* "); line_offset.advance(b"/* "); - j.push_static(pretty); - line_offset.advance(pretty); + // Escape `*/` so a hostile package path can't terminate the + // comment and inject arbitrary CSS into the bundle output. + if bun_core::immutable::contains(pretty, b"*/") { + let mut escaped: Vec = Vec::with_capacity(pretty.len() + 1); + let mut i = 0; + while i < pretty.len() { + if pretty[i] == b'*' && pretty.get(i + 1) == Some(&b'/') { + escaped.extend_from_slice(b"*\\/"); + i += 2; + } else { + escaped.push(pretty[i]); + i += 1; + } + } + line_offset.advance(&escaped); + j.push_owned(escaped.into_boxed_slice()); + } else { + j.push_static(pretty); + line_offset.advance(pretty); + } j.push_static(b" */\n"); line_offset.advance(b" */\n"); diff --git a/src/bundler/linker_context/postProcessJSChunk.rs b/src/bundler/linker_context/postProcessJSChunk.rs index 6d29cc03a97..f3b197b6e47 100644 --- a/src/bundler/linker_context/postProcessJSChunk.rs +++ b/src/bundler/linker_context/postProcessJSChunk.rs @@ -681,8 +681,26 @@ pub fn post_process_js_chunk( } } - j.push_static(pretty); - line_offset.advance(pretty); + // Escape `*/` so a hostile package path can't terminate the multiline + // comment and inject JS. (Single-line is safe: no line terminators above.) + if matches!(comment_type, CommentType::Multiline) && strings::contains(pretty, b"*/") { + let mut escaped: Vec = Vec::with_capacity(pretty.len() + 1); + let mut i = 0; + while i < pretty.len() { + if pretty[i] == b'*' && pretty.get(i + 1) == Some(&b'/') { + escaped.extend_from_slice(b"*\\/"); + i += 2; + } else { + escaped.push(pretty[i]); + i += 1; + } + } + line_offset.advance(&escaped); + j.push_owned(escaped.into_boxed_slice()); + } else { + j.push_static(pretty); + line_offset.advance(pretty); + } if emit_targets_in_commands { j.push_static(b" ("); diff --git a/src/bundler/options.rs b/src/bundler/options.rs index 2ce4d504614..a0758304b70 100644 --- a/src/bundler/options.rs +++ b/src/bundler/options.rs @@ -2600,10 +2600,21 @@ pub(crate) fn path_template_print( }; match field { - PlaceholderField::Dir => PathTemplate::write_replacing_slashes_on_windows( - writer, - if !dir.is_empty() { dir } else { b"." }, - )?, + PlaceholderField::Dir => { + // Rewrite leading `..` segments to `_.._` so `[dir]` can't place + // output above `outdir` (mirrors esbuild's `outbase` behavior). + let mut d: &[u8] = if !dir.is_empty() { dir } else { b"." }; + while d.starts_with(b"../") || d.starts_with(b"..\\") { + writer.write_all(b"_.._")?; + PathTemplate::write_replacing_slashes_on_windows(writer, &d[2..3])?; + d = &d[3..]; + } + if d == b".." { + writer.write_all(b"_.._")?; + } else { + PathTemplate::write_replacing_slashes_on_windows(writer, d)?; + } + } PlaceholderField::Name => { PathTemplate::write_replacing_slashes_on_windows(writer, name)? } diff --git a/src/css/css_parser.rs b/src/css/css_parser.rs index bee67351cc6..8269b677ef7 100644 --- a/src/css/css_parser.rs +++ b/src/css/css_parser.rs @@ -668,7 +668,16 @@ fn parse_nested_block( let saved_stop_before = parser.stop_before; parser.stop_before = closing_delimiter; parser.at_start_of = None; - let result = parser.parse_entirely((), |(), p| parsefn(p)); + // Bound recursive descent so deeply nested blocks (e.g. `calc(calc(...))`) + // can't overflow the native stack; the error path leaves the parser recoverable. + let result = if parser.nesting_depth >= MAX_NESTING_DEPTH { + Err(parser.new_custom_error(ParserError::maximum_nesting_depth)) + } else { + parser.nesting_depth += 1; + let r = parser.parse_entirely((), |(), p| parsefn(p)); + parser.nesting_depth -= 1; + r + }; if let Some(block_type2) = parser.at_start_of.take() { consume_until_end_of_block(block_type2, &mut parser.input.tokenizer); } @@ -3439,8 +3448,14 @@ pub struct Parser<'a> { /// materialises a fresh short-lived `&mut` instead. pub import_records: Option>>, pub extra: Option<&'a mut ParserExtra>, + /// Current nested-block depth; bounded by [`MAX_NESTING_DEPTH`] to prevent + /// stack overflow from deeply nested input. + pub nesting_depth: u32, } +/// Hard cap on nested-block depth (matches WebKit's CSS tokenizer limit). +pub const MAX_NESTING_DEPTH: u32 = 128; + impl<'a> Parser<'a> { pub fn add_symbol_for_name( &mut self, @@ -3563,6 +3578,7 @@ impl<'a> Parser<'a> { flags, import_records, extra, + nesting_depth: 0, } } diff --git a/src/http_jsc/websocket_client.rs b/src/http_jsc/websocket_client.rs index 9701b48fbee..2f48866412c 100644 --- a/src/http_jsc/websocket_client.rs +++ b/src/http_jsc/websocket_client.rs @@ -310,16 +310,21 @@ impl WebSocket { // null is well-defined (BoringSSL returns null). let servername = unsafe { boringssl::c::SSL_get_servername(ssl_ptr, TLSEXT_NAMETYPE_HOST_NAME) }; - if !servername.is_null() { + // SNI may be null (e.g. IP literal); with no hostname there is nothing + // to verify the certificate against, so reject rather than skip verification. + let hostname: &[u8] = if !servername.is_null() { // SAFETY: servername is a NUL-terminated C string owned by the SSL session. - let hostname = unsafe { bun_core::ffi::cstr(servername) }.to_bytes(); - // SAFETY: ssl_ptr is non-null (connected SSL socket on the handshake path). - if !ssl_ptr.is_null() - && !boringssl::check_server_identity(unsafe { &mut *ssl_ptr }, hostname) - { - self.outgoing_websocket = None; - ws_ref.did_abrupt_close(ErrorCode::FailedToConnect); - } + unsafe { bun_core::ffi::cstr(servername) }.to_bytes() + } else { + b"" + }; + if ssl_ptr.is_null() + || hostname.is_empty() + // SAFETY: ssl_ptr is non-null (checked above) for the open socket's lifetime. + || !boringssl::check_server_identity(unsafe { &mut *ssl_ptr }, hostname) + { + self.outgoing_websocket = None; + ws_ref.did_abrupt_close(ErrorCode::FailedToConnect); } } // If reject_unauthorized is false, we accept the connection regardless of SSL errors diff --git a/src/install/bin.rs b/src/install/bin.rs index 5b3250f2b74..0a3d06af702 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -737,6 +737,17 @@ pub fn normalized_bin_name(name: &[u8]) -> &[u8] { name } +/// True when `abs_target` is contained in `package_dir`. `bin` targets are +/// untrusted package.json values; a `../../..` target would be symlinked + chmod 0o777. +fn bin_target_within_package_dir(abs_target: &[u8], package_dir: &[u8]) -> bool { + debug_assert!(matches!(package_dir.last(), Some(&b'/') | Some(&b'\\'))); + let dir = strings::without_trailing_slash(package_dir); + if !strings::has_prefix(abs_target, dir) { + return false; + } + matches!(abs_target.get(dir.len()), None | Some(&b'/') | Some(&b'\\')) +} + pub struct Linker<'a> { pub bin: Bin, @@ -1436,6 +1447,11 @@ impl<'a> Linker<'a> { target, unscoped_package_name, ); + // SECURITY: refuse `bin` targets that traverse outside the + // package directory (would otherwise be symlinked + chmod 0o777). + if !bin_target_within_package_dir(r.as_bytes(), package_dir) { + return; + } // SAFETY: `resolve_bin_target` writes into the thread-local // `PARSER_JOIN_INPUT_BUFFER` (via `join_abs_string_z`); the // returned slice does not actually borrow `self` or @@ -1476,6 +1492,11 @@ impl<'a> Linker<'a> { target, normalized_name, ); + // SECURITY: refuse `bin` targets that traverse outside the + // package directory (would otherwise be symlinked + chmod 0o777). + if !bin_target_within_package_dir(r.as_bytes(), package_dir) { + return; + } // SAFETY: thread-local buffer; see Tag::File above. ZStr::from_raw(r.as_bytes().as_ptr(), r.len()) }; @@ -1521,6 +1542,12 @@ impl<'a> Linker<'a> { bin_target, normalized_bin_dest, ); + // SECURITY: refuse `bin` targets that traverse outside the + // package directory (would otherwise be symlinked + chmod 0o777). + if !bin_target_within_package_dir(r.as_bytes(), package_dir) { + i += 2; + continue; + } // SAFETY: thread-local buffer; see Tag::File above. ZStr::from_raw(r.as_bytes().as_ptr(), r.len()) }; @@ -1551,6 +1578,11 @@ impl<'a> Linker<'a> { let package_dir = &self.abs_target_buf[0..package_dir_len]; let r = resolve_path::join_abs_string_z::(package_dir, &[target]); + // SECURITY: refuse `directories.bin` targets that traverse + // outside the package directory. + if !bin_target_within_package_dir(r.as_bytes(), package_dir) { + return; + } // SAFETY: `join_abs_string_z` writes into the thread-local // `PARSER_JOIN_INPUT_BUFFER`; result does not borrow // `package_dir`. Detached so `abs_target_buf` can be diff --git a/src/js/bun/sql.ts b/src/js/bun/sql.ts index dc436d367fe..9edd2dd37c1 100644 --- a/src/js/bun/sql.ts +++ b/src/js/bun/sql.ts @@ -744,8 +744,11 @@ const SQL: typeof Bun.SQL = function SQL( if (!$isCallable(savepoint_callback)) { throw $ERR_INVALID_ARG_VALUE("fn", callback, "must be a function"); } - // matchs the format of the savepoint name in postgres package - const save_point_name = `s${savepoints++}${name ? `_${name}` : ""}`; + // matchs the format of the savepoint name in postgres package. + // Strip non-[A-Za-z0-9_] from the user-supplied name; it is interpolated + // unquoted into SAVEPOINT / RELEASE / ROLLBACK statements. + const safe_name = name ? String(name).replace(/[^A-Za-z0-9_]/g, "") : ""; + const save_point_name = `s${savepoints++}${safe_name ? `_${safe_name}` : ""}`; const promise = run_internal_savepoint(save_point_name, savepoint_callback); transactionSavepoints.add(promise); return await promise.finally(onSavepointFinished.bind(null, promise)); diff --git a/src/js/internal/debugger.ts b/src/js/internal/debugger.ts index dab0013d340..2aacf459122 100644 --- a/src/js/internal/debugger.ts +++ b/src/js/internal/debugger.ts @@ -592,7 +592,9 @@ function parseUrl(input: string): URL { } function randomId() { - return Math.random().toString(36).slice(2); + // Sole auth token for the debugger WebSocket — must be unpredictable. + // Strip hyphens to keep the historical `[a-z0-9]+` path shape. + return crypto.randomUUID().replace(/-/g, ""); } const { enableANSIColors } = Bun; diff --git a/src/js/internal/sql/mysql.ts b/src/js/internal/sql/mysql.ts index aa66d74655e..2894be5c83a 100644 --- a/src/js/internal/sql/mysql.ts +++ b/src/js/internal/sql/mysql.ts @@ -539,7 +539,15 @@ class MySQLAdapter }; } - validateTransactionOptions(_options: string): { valid: boolean; error?: string } { + validateTransactionOptions(options: string): { valid: boolean; error?: string } { + // Options are interpolated unquoted into `START TRANSACTION ${options}`; + // restrict to letters/spaces/commas to prevent statement injection. + if (options && !/^[a-zA-Z, ]+$/.test(options)) { + return { + valid: false, + error: "Transaction options may only contain letters, spaces, and commas.", + }; + } return { valid: true }; } diff --git a/src/js/internal/sql/postgres.ts b/src/js/internal/sql/postgres.ts index efc3499b01d..6b0b4f2471c 100644 --- a/src/js/internal/sql/postgres.ts +++ b/src/js/internal/sql/postgres.ts @@ -204,13 +204,20 @@ function arrayValueSerializer(type: ArrayType, is_numeric: boolean, is_json: boo return `"${arrayEscape(JSON.stringify(value))}"`; } } +// `arrayType` is interpolated unquoted as `$N::[]`; restrict to +// PostgreSQL type-name characters to prevent SQL injection. +const VALID_ARRAY_TYPE_RE = /^[A-Z_][A-Z0-9_]*(?:[ .][A-Z_][A-Z0-9_]*)*$/; function getArrayType(typeNameOrID: number | ArrayType | undefined = undefined): ArrayType { const typeOfType = typeof typeNameOrID; if (typeOfType === "number") { return getPostgresArrayType(typeNameOrID as number) ?? "JSON"; } if (typeOfType === "string") { - return (typeNameOrID as string)?.toUpperCase(); + const arrayType = (typeNameOrID as string).toUpperCase(); + if (!VALID_ARRAY_TYPE_RE.test(arrayType)) { + throw new Error(`Invalid PostgreSQL array type: ${JSON.stringify(typeNameOrID)}`); + } + return arrayType; } // default to JSON so we accept most of the types return "JSON"; @@ -766,8 +773,15 @@ class PostgresAdapter }; } - validateTransactionOptions(_options: string): { valid: boolean; error?: string } { - // PostgreSQL accepts any transaction options + validateTransactionOptions(options: string): { valid: boolean; error?: string } { + // Options are interpolated unquoted into `BEGIN ${options}` (simple-query + // protocol allows stacked statements); restrict to letters/spaces/commas. + if (options && !/^[a-zA-Z, ]+$/.test(options)) { + return { + valid: false, + error: "Transaction options may only contain letters, spaces, and commas.", + }; + } return { valid: true }; } diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index 235f78a1214..545b37d9d42 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -686,6 +686,15 @@ class SQLiteAdapter implements DatabaseAdapter(data), byteLength, byteLength, deserializeFlags); + // SQLITE_DESERIALIZE_FREEONCLOSE means sqlite3_deserialize() already frees + // `data` on failure; freeing it again here would be a double-free. if (status == SQLITE_BUSY) { - sqlite3_free(data); throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "SQLITE_BUSY"_s)); return {}; } if (status != SQLITE_OK) { - sqlite3_free(data); throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, status == SQLITE_ERROR ? "unable to deserialize database"_s : sqliteString(sqlite3_errstr(status)))); return {}; } @@ -1804,7 +1804,15 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementFcntlFunction, (JSC::JSGlobalObject * lex RETURN_IF_EXCEPTION(scope, {}); } - int resultInt = -1; + // sqlite3_file_control() may write a pointer/int64/pointer-array through pArg + // depending on opcode; a 4-byte stack int would be a stack overflow. Reserve + // a max-aligned buffer wide enough for any documented opcode. + union { + int i; + sqlite3_int64 i64; + void* ptr; + unsigned char raw[64]; + } numericScratch = {}; void* resultPtr = nullptr; if (resultValue.isObject()) { if (auto* view = dynamicDowncast(resultValue.getObject())) { @@ -1818,12 +1826,52 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementFcntlFunction, (JSC::JSGlobalObject * lex throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Expected buffer"_s)); return {}; } + + // These opcodes write through pArg; reject undersized typed arrays. + switch (op) { + // 8-byte outputs (pointer / sqlite3_int64 / int[2]). + case SQLITE_FCNTL_FILE_POINTER: + case SQLITE_FCNTL_VFS_POINTER: + case SQLITE_FCNTL_JOURNAL_POINTER: + case SQLITE_FCNTL_VFSNAME: + case SQLITE_FCNTL_TEMPFILENAME: + case SQLITE_FCNTL_GET_LOCKPROXYFILE: + case SQLITE_FCNTL_WIN32_GET_HANDLE: + case SQLITE_FCNTL_WIN32_AV_RETRY: + case SQLITE_FCNTL_SIZE_HINT: + case SQLITE_FCNTL_MMAP_SIZE: + case SQLITE_FCNTL_SIZE_LIMIT: { + constexpr size_t minSize = sizeof(void*) > sizeof(sqlite3_int64) ? sizeof(void*) : sizeof(sqlite3_int64); + if (view->byteLength() < minSize) { + throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "TypedArray is too small for this file control opcode"_s)); + return {}; + } + break; + } + // `int` outputs (4 bytes). + case SQLITE_FCNTL_LOCKSTATE: + case SQLITE_FCNTL_LAST_ERRNO: + case SQLITE_FCNTL_PERSIST_WAL: + case SQLITE_FCNTL_POWERSAFE_OVERWRITE: + case SQLITE_FCNTL_HAS_MOVED: + case SQLITE_FCNTL_DATA_VERSION: + case SQLITE_FCNTL_RESERVE_BYTES: + case SQLITE_FCNTL_EXTERNAL_READER: { + if (view->byteLength() < sizeof(int)) { + throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "TypedArray is too small for this file control opcode"_s)); + return {}; + } + break; + } + default: + break; + } } } else if (resultValue.isNumber()) { - resultInt = resultValue.toInt32(lexicalGlobalObject); + numericScratch.i = resultValue.toInt32(lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, {}); - resultPtr = &resultInt; + resultPtr = &numericScratch; } else if (resultValue.isNull()) { } else { diff --git a/src/jsc/bindings/webcore/JSDOMFormData.cpp b/src/jsc/bindings/webcore/JSDOMFormData.cpp index 6a82857015f..175802a1120 100644 --- a/src/jsc/bindings/webcore/JSDOMFormData.cpp +++ b/src/jsc/bindings/webcore/JSDOMFormData.cpp @@ -646,8 +646,12 @@ JSC::JSValue getInternalProperties(JSC::VM& vm, JSGlobalObject* lexicalGlobalObj auto& key = entry.name; auto& value = entry.data; auto ident = Identifier::fromString(vm, key); + // Numeric-string keys land in indexed storage via putDirectMayBeIndex; + // getDirect() doesn't see them, so use getDirectIndex on the read side. + std::optional index = parseIndex(ident); if (seenKeys.contains(key)) { - JSValue jsValue = obj->getDirect(vm, ident); + JSValue jsValue = index ? obj->getDirectIndex(lexicalGlobalObject, index.value()) : obj->getDirect(vm, ident); + RETURN_IF_EXCEPTION(throwScope, {}); if (jsValue.isString() || jsValue.inherits()) { // Make sure this runs before the deferral scope is called. JSValue resultValue = toJSValue(value); @@ -670,7 +674,8 @@ JSC::JSValue getInternalProperties(JSC::VM& vm, JSGlobalObject* lexicalGlobalObj array->initializeIndex(initializationScope, 1, resultValue); } - obj->putDirect(vm, ident, array, 0); + obj->putDirectMayBeIndex(lexicalGlobalObject, ident, array); + RETURN_IF_EXCEPTION(throwScope, {}); } else if (jsValue.isObject() && jsValue.getObject()->inherits()) { JSC::JSArray* array = uncheckedDowncast(jsValue.getObject()); JSValue jsValue = toJSValue(value); diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index f1e72f3fa24..ef13d130550 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/src/jsc/bindings/webcore/SerializedScriptValue.cpp @@ -3972,9 +3972,13 @@ class CloneDeserializer : public CloneBase { bool read(BIO** bio, uint64_t length) { - if (m_ptr + length > m_end) + // Compare remaining bytes (not `m_ptr + length`) to avoid pointer overflow. + if (length > static_cast(m_end - m_ptr)) + return false; + // BIO_new_mem_buf takes `int`; reject lengths that would truncate or go negative. + if (length > static_cast(std::numeric_limits::max())) return false; - *bio = BIO_new_mem_buf(m_ptr, length); + *bio = BIO_new_mem_buf(m_ptr, static_cast(length)); if (!*bio) return false; m_ptr += length; diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 6651e49ea3c..50eb6271921 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -1617,6 +1617,11 @@ impl Archiver { let mut symlink_join_buf: Option = None; // (guard Drop puts the buffer back to the pool) + // Paths of symlinks created during this extraction; later entries that + // traverse through one are rejected to prevent chained-symlink escape. + #[cfg(unix)] + let mut created_symlinks: Vec> = Vec::new(); + let mut normalized_buf = OSPathBuffer::uninit(); let mut use_pwrite = cfg!(unix); let mut use_lseek = true; @@ -1787,6 +1792,36 @@ impl Archiver { count += 1; + // Reject entries whose path passes through (or equals) a + // symlink created earlier in this extraction (chained-symlink escape). + #[cfg(unix)] + { + // Case-insensitive on macOS (APFS default is case-insensitive). + #[cfg(target_os = "macos")] + fn component_eq(a: &[u8], b: &[u8]) -> bool { + a.eq_ignore_ascii_case(b) + } + #[cfg(all(unix, not(target_os = "macos")))] + fn component_eq(a: &[u8], b: &[u8]) -> bool { + a == b + } + let traverses_created_symlink = created_symlinks.iter().any(|prefix| { + path_slice.len() >= prefix.len() + && component_eq(&path_slice[..prefix.len()], &prefix[..]) + && (path_slice.len() == prefix.len() + || path_slice[prefix.len()] == b'/') + }); + if traverses_created_symlink { + if options.log { + Output::warn(&format_args!( + "Skipping entry that traverses a previously created symlink: {}\n", + bun_core::fmt::fmt_os_path(path_slice, Default::default()), + )); + } + continue; + } + } + match kind { bun_sys::FileKind::Directory => { // SAFETY: entry valid @@ -1889,6 +1924,9 @@ impl Archiver { _ => return Err(err.into()), }, } + // Remember that this path is a symlink so later + // entries cannot be written through it. + created_symlinks.push(path_slice.to_vec()); } #[cfg(not(unix))] { diff --git a/src/parsers/yaml.rs b/src/parsers/yaml.rs index e9fb12489b3..b80f939cc5d 100644 --- a/src/parsers/yaml.rs +++ b/src/parsers/yaml.rs @@ -3096,15 +3096,28 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { pub struct MappingProps { list: G::PropertyList, + /// Pointers of `E::Object`s already folded in via `<<` merge keys, so + /// repeated `*anchor` references aren't re-merged (avoids cubic blowup). + merged_objects: Vec<*const E::Object>, } impl MappingProps { pub fn init() -> Self { Self { list: bun_alloc::AstAlloc::vec(), + merged_objects: Vec::new(), } } + /// Records `obj` and returns `true` if not seen before (caller skips on `false`). + fn note_merged(&mut self, obj: *const E::Object) -> bool { + if self.merged_objects.contains(&obj) { + return false; + } + self.merged_objects.push(obj); + true + } + pub fn append(&mut self, prop: G::Property) -> Result<(), AllocError> { self.list.push(prop); Ok(()) @@ -3154,13 +3167,21 @@ impl MappingProps { } match &value.data { - ast::ExprData::EObject(value_obj) => self.merge(value_obj.properties.slice()), + ast::ExprData::EObject(value_obj) => { + if !self.note_merged(value_obj.as_ptr()) { + return Ok(()); + } + self.merge(value_obj.properties.slice()) + } ast::ExprData::EArray(value_arr) => { for item in value_arr.items.slice() { let item_obj = match &item.data { ast::ExprData::EObject(obj) => obj, _ => continue, }; + if !self.note_merged(item_obj.as_ptr()) { + continue; + } self.merge(item_obj.properties.slice())?; } Ok(()) diff --git a/src/patch/lib.rs b/src/patch/lib.rs index a989f293e04..24f2508a8e5 100644 --- a/src/patch/lib.rs +++ b/src/patch/lib.rs @@ -112,6 +112,11 @@ impl<'a> PatchFile<'a> { } } PatchFilePart::FileRename(file_rename) => { + if !is_safe_patch_path(file_rename.from_path) + || !is_safe_patch_path(file_rename.to_path) + { + return Some(sys::Error::from_code(sys::E::EINVAL, sys::Tag::rename)); + } let from_path = ZBox::from_vec_with_nul(file_rename.from_path.to_vec()); let to_path = ZBox::from_vec_with_nul(file_rename.to_path.to_vec()); @@ -216,12 +221,18 @@ impl<'a> PatchFile<'a> { } } PatchFilePart::FilePatch(file_patch) => { + if !is_safe_patch_path(file_patch.path) { + return Some(sys::Error::from_code(sys::E::EINVAL, sys::Tag::open)); + } // TODO: should we compute the hash of the original file and check it against the on in the patch? if let sys::Result::Err(e) = apply_patch(file_patch, patch_dir, &mut state) { return Some(e.without_path()); } } PatchFilePart::FileModeChange(file_mode_change) => { + if !is_safe_patch_path(file_mode_change.path) { + return Some(sys::Error::from_code(sys::E::EINVAL, sys::Tag::fchmodat)); + } let newmode = file_mode_change.new_mode; let filepath = ZBox::from_vec_with_nul(file_mode_change.path.to_vec()); #[cfg(unix)] diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index 63ca7af8dc3..843e58b4015 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -2738,6 +2738,19 @@ impl<'a> ESModule<'a> { if PATTERN { // Return the URL resolution of resolvedTarget with every instance of "*" replaced with subpath. let len = replacement_size(str, b"*", subpath); + // Untrusted target/subpath: many `*`s × a long subpath + // can exceed the fixed PathBuffer; bail instead of overflowing. + if len > resolve_target_buf2.0.len() { + dedent!(); + return Resolution { + path: Box::<[u8]>::from(str), + status: Status::InvalidPackageTarget, + debug: ResolutionDebug { + token: target.first_token, + ..Default::default() + }, + }; + } let _ = replace(str, b"*", subpath, &mut resolve_target_buf2.0); let result = &resolve_target_buf2.0[0..len]; if let Some(log) = self.debug_logs.as_deref_mut() { @@ -2845,6 +2858,18 @@ impl<'a> ESModule<'a> { if PATTERN { // Return the URL resolution of resolvedTarget with every instance of "*" replaced with subpath. let len = replacement_size(resolved_target, b"*", subpath); + // Same untrusted-input bound as above. + if len > resolve_target_buf2.0.len() { + dedent!(); + return Resolution { + path: Box::<[u8]>::from(str), + status: Status::InvalidPackageTarget, + debug: ResolutionDebug { + token: target.first_token, + ..Default::default() + }, + }; + } let _ = replace(resolved_target, b"*", subpath, &mut resolve_target_buf2.0); let result = &resolve_target_buf2.0[0..len]; if let Some(log) = self.debug_logs.as_deref_mut() { @@ -3302,6 +3327,8 @@ fn find_invalid_segment(path_: &[u8]) -> Option<&[u8]> { path = b""; } + // Mirrors Node's `invalidSegmentRegEx`: percent-encoded dots must be + // rejected too, since the path is percent-decoded later in `finalize()`. match segment.len() { 1 => { if segment == b"." { @@ -3313,6 +3340,26 @@ fn find_invalid_segment(path_: &[u8]) -> Option<&[u8]> { return Some(segment); } } + 3 => { + // "%2e" + if strings::eql_case_insensitive_ascii(segment, b"%2e", true) { + return Some(segment); + } + } + 4 => { + // ".%2e" / "%2e." + if strings::eql_case_insensitive_ascii(segment, b".%2e", true) + || strings::eql_case_insensitive_ascii(segment, b"%2e.", true) + { + return Some(segment); + } + } + 6 => { + // "%2e%2e" + if strings::eql_case_insensitive_ascii(segment, b"%2e%2e", true) { + return Some(segment); + } + } 12 => { // "node_modules".len if segment == b"node_modules" { diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index bdfa1dafa71..ac739cbf42a 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -4028,19 +4028,36 @@ impl H2FrameParser { ); return Ok(data.len()); } + + let settings = self + .remote_settings + .get() + .unwrap_or(self.local_settings.get()); + if frame.length > settings.max_frame_size { + self.send_go_away( + frame.stream_identifier, + ErrorCode::FRAME_SIZE_ERROR, + b"invalid Continuation frame size", + self.last_stream_id.get(), + true, + ); + return Ok(data.len()); + } + if let Some(content) = self.handle_incomming_payload(data, frame.stream_identifier) { let payload = content.data(); let end = content.end; self.read_buffer.with_mut(|rb| rb.reset()); - stream.end_after_headers = frame.flags & HeadersFrameFlags::END_STREAM as u8 != 0; stream = match self.decode_header_block(payload, stream, frame.flags)? { // SAFETY: s is *mut Stream from self.streams (heap::alloc); valid while the map entry exists Some(s) => unsafe { &mut *s }, None => return Ok(end), }; - if stream.end_after_headers { + // RFC 9113 §6.10: CONTINUATION carries END_HEADERS only; END_STREAM + // was captured from the originating HEADERS frame in `end_after_headers`. + if frame.flags & HeadersFrameFlags::END_HEADERS as u8 != 0 { stream.is_waiting_more_headers = false; - if frame.flags & HeadersFrameFlags::END_STREAM as u8 != 0 { + if stream.end_after_headers { let identifier = stream.get_identifier(); identifier.ensure_still_alive(); if stream.state == StreamState::HALF_CLOSED_REMOTE { @@ -4465,6 +4482,37 @@ impl H2FrameParser { Some(stream) } + /// Like [`Self::handle_received_stream_id`], but bounds new-stream allocation + /// from inbound frames so a peer can't force unbounded `Stream` creation (RFC 7540 §5.1.2). + fn handle_remote_stream_id(&self, stream_identifier: u32) -> Option<*mut Stream> { + // Default SETTINGS_MAX_CONCURRENT_STREAMS is "unlimited"; apply a cap when + // the user hasn't explicitly configured `maxConcurrentStreams`. + const MAX_REMOTE_STREAMS_DEFAULT_CAP: u32 = 1000; + if stream_identifier != 0 && !self.streams.get().contains_key(&stream_identifier) { + let configured = self.local_settings.get().max_concurrent_streams; + let max = if configured == u32::MAX { + MAX_REMOTE_STREAMS_DEFAULT_CAP + } else { + configured + }; + // Count open streams; CLOSED ones stay in the map for pointer stability. + let mut open: u32 = 0; + for (_, stream) in self.streams.get().iter() { + // SAFETY: boxed Stream outlives the iteration (entries are never removed). + if unsafe { (**stream).state } != StreamState::CLOSED { + open += 1; + if open >= max { + return None; + } + } + } + if max == 0 { + return None; + } + } + self.handle_received_stream_id(stream_identifier) + } + fn read_bytes(&self, bytes: &[u8]) -> JsResult { bun_output::scoped_log!(H2FrameParser, "read {}", bytes.len()); if self.is_server.get() && self.preface_received_len.get() < 24 { @@ -4507,7 +4555,7 @@ impl H2FrameParser { header.stream_identifier ); - let stream = self.handle_received_stream_id(header.stream_identifier); + let stream = self.handle_remote_stream_id(header.stream_identifier); return self.dispatch_frame(header, bytes, stream, 0); } @@ -4554,7 +4602,7 @@ impl H2FrameParser { header.flags, header.stream_identifier ); - let stream = self.handle_received_stream_id(header.stream_identifier); + let stream = self.handle_remote_stream_id(header.stream_identifier); return self.dispatch_frame(header, &bytes[needed..], stream, needed); } @@ -4589,7 +4637,7 @@ impl H2FrameParser { ); self.current_frame.set(Some(header)); self.remaining_length.set(header.length as i32); - let stream = self.handle_received_stream_id(header.stream_identifier); + let stream = self.handle_remote_stream_id(header.stream_identifier); self.dispatch_frame( header, &bytes[FrameHeader::BYTE_SIZE..], @@ -7144,12 +7192,18 @@ impl H2FrameParser { if end_stream { stream.end_after_headers = true; - stream.state = StreamState::HALF_CLOSED_LOCAL; - if wait_for_trailers { + stream.state = StreamState::HALF_CLOSED_LOCAL; this.dispatch(JSH2FrameParser::Gc::onWantTrailers, stream.get_identifier()); return Ok(JSValue::js_number(stream_id as f64)); } + // RFC 7540 §5.1: both sides END_STREAM → CLOSED, not HALF_CLOSED_LOCAL + // (mirrors `flush_queue_after_data_frame`). + if stream.state == StreamState::HALF_CLOSED_REMOTE { + stream.state = StreamState::CLOSED; + } else { + stream.state = StreamState::HALF_CLOSED_LOCAL; + } } else { stream.wait_for_trailers = wait_for_trailers; } diff --git a/src/runtime/bake/DevServer/HmrSocket.rs b/src/runtime/bake/DevServer/HmrSocket.rs index 4c0aa7bfb65..4e9f7168228 100644 --- a/src/runtime/bake/DevServer/HmrSocket.rs +++ b/src/runtime/bake/DevServer/HmrSocket.rs @@ -19,6 +19,27 @@ pub use super::ResponseLike; // single type (no cross-type pointer casts). pub use super::HmrSocket; +/// Echoes browser-relayed `console.log` payloads to the terminal with control +/// bytes (ESC/CSI/OSC, C0/C1) stripped to prevent terminal escape injection. +struct SanitizedBrowserLog<'a>(&'a [u8]); + +impl core::fmt::Display for SanitizedBrowserLog<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + use bstr::ByteSlice; + use core::fmt::Write; + for ch in self.0.chars() { + // Drop C0 (except `\n`/`\t`), DEL, and C1 controls. + let is_control = matches!(ch, '\u{00}'..='\u{1f}' | '\u{7f}'..='\u{9f}'); + if is_control && ch != '\n' && ch != '\t' { + f.write_char('\u{fffd}')?; + } else { + f.write_char(ch)?; + } + } + Ok(()) + } +} + impl HmrSocket { // `res: anytype` — only `.getRemoteSocketInfo()` is called on it. // Bound matches the caller in `DevServer::on_web_socket_upgrade`. @@ -295,13 +316,13 @@ impl HmrSocket { ConsoleLogKind::Log => { Output::pretty(format_args!( "[browser] {}\n", - bstr::BStr::new(data) + SanitizedBrowserLog(data) )); } ConsoleLogKind::Err => { Output::pretty_error(format_args!( "[browser] {}\n", - bstr::BStr::new(data) + SanitizedBrowserLog(data) )); } } diff --git a/src/runtime/cli/pm_view_command.rs b/src/runtime/cli/pm_view_command.rs index 080099992f0..0b0bfe151ef 100644 --- a/src/runtime/cli/pm_view_command.rs +++ b/src/runtime/cli/pm_view_command.rs @@ -424,19 +424,19 @@ pub fn view( prettyln!( "{}@{} | {} | deps: {} | versions: {}", - BStr::new(pkg_name), - BStr::new(pkg_version), - BStr::new(license), + Sanitized(pkg_name), + Sanitized(pkg_version), + Sanitized(license), dep_count, versions_len, ); // Get description and homepage from the top-level package manifest, not the version-specific one if let Some(desc) = json.get_string_cloned(&bump, b"description").ok().flatten() { - prettyln!("{}", BStr::new(desc)); + prettyln!("{}", Sanitized(desc)); } if let Some(hp) = json.get_string_cloned(&bump, b"homepage").ok().flatten() { - prettyln!("{}", BStr::new(hp)); + prettyln!("{}", Sanitized(hp)); } if let Some(mut iter) = json.get_array(b"keywords") { @@ -453,7 +453,7 @@ pub fn view( } } if !keywords.list.is_empty() { - prettyln!("keywords: {}", BStr::new(keywords.list.as_slice())); + prettyln!("keywords: {}", Sanitized(keywords.list.as_slice())); } } @@ -487,8 +487,8 @@ pub fn view( }; prettyln!( "- {}: {}", - BStr::new(dep_name), - BStr::new(dep_version), + Sanitized(dep_name), + Sanitized(dep_version), ); } } @@ -496,13 +496,13 @@ pub fn view( if let Some(dist) = manifest.get_object(b"dist") { prettyln!("\ndist"); if let Some(t) = dist.get_string_cloned(&bump, b"tarball").ok().flatten() { - prettyln!(" .tarball: {}", BStr::new(t)); + prettyln!(" .tarball: {}", Sanitized(t)); } if let Some(s) = dist.get_string_cloned(&bump, b"shasum").ok().flatten() { - prettyln!(" .shasum: {}", BStr::new(s)); + prettyln!(" .shasum: {}", Sanitized(s)); } if let Some(i) = dist.get_string_cloned(&bump, b"integrity").ok().flatten() { - prettyln!(" .integrity: {}", BStr::new(i)); + prettyln!(" .integrity: {}", Sanitized(i)); } if let Some(u) = dist.get_number(b"unpackedSize") { prettyln!( @@ -529,11 +529,11 @@ pub fn view( if let Some(tag) = tagname_expr.as_string(&bump) { if let Some(val) = val_expr.as_string(&bump) { if tag == b"latest" { - prettyln!("{}: {}", BStr::new(tag), BStr::new(val)); + prettyln!("{}: {}", Sanitized(tag), Sanitized(val)); } else if tag == b"beta" { - prettyln!("{}: {}", BStr::new(tag), BStr::new(val)); + prettyln!("{}: {}", Sanitized(tag), Sanitized(val)); } else { - prettyln!("{}: {}", BStr::new(tag), BStr::new(val)); + prettyln!("{}: {}", Sanitized(tag), Sanitized(val)); } } } @@ -554,9 +554,9 @@ pub fn view( .flatten() .unwrap_or(b""); if !em.is_empty() { - prettyln!("- {} \\<{}\\>", BStr::new(nm), BStr::new(em)); + prettyln!("- {} \\<{}\\>", Sanitized(nm), Sanitized(em)); } else if !nm.is_empty() { - prettyln!("- {}", BStr::new(nm)); + prettyln!("- {}", Sanitized(nm)); } } } @@ -569,17 +569,36 @@ pub fn view( .ok() .flatten() { - prettyln!("\nPublished: {}", BStr::new(published_time)); + prettyln!("\nPublished: {}", Sanitized(published_time)); } else if let Some(modified_time) = time_obj .get_string_cloned(&bump, b"modified") .ok() .flatten() { - prettyln!("\nPublished: {}", BStr::new(modified_time)); + prettyln!("\nPublished: {}", Sanitized(modified_time)); } } Ok(()) } +/// Displays registry-derived metadata with control characters stripped so +/// malicious package fields can't inject terminal escape sequences. +struct Sanitized<'a>(&'a [u8]); + +impl core::fmt::Display for Sanitized<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + use bstr::ByteSlice; + use core::fmt::Write; + // Lossy-decode UTF-8 and drop control chars except `\n` / `\t`. + for ch in self.0.chars() { + if ch.is_control() && ch != '\n' && ch != '\t' { + continue; + } + f.write_char(ch)?; + } + Ok(()) + } +} + // ported from: src/cli/pm_view_command.zig diff --git a/src/runtime/cli/upgrade_command.rs b/src/runtime/cli/upgrade_command.rs index 672db4178c8..268ee9ef355 100644 --- a/src/runtime/cli/upgrade_command.rs +++ b/src/runtime/cli/upgrade_command.rs @@ -722,6 +722,21 @@ impl UpgradeCommand { let version_name = version.name().unwrap(); + // Random per-invocation subdir name; a predictable `//` + // lets another local user pre-stage the path and tamper with the binary. + let tmp_subdir: Vec = { + let mut out = Vec::with_capacity(version_name.len() + 1 + 16); + out.extend_from_slice(&version_name); + out.push(b'-'); + write!( + &mut out, + "{}", + bun_fmt::hex_int_lower::<16>(bun_core::fast_random()), + ) + .expect("oom"); + out + }; + let save_dir_: sys::Dir = match filesystem.tmpdir() { Ok(d) => d, Err(err) => { @@ -730,7 +745,7 @@ impl UpgradeCommand { } }; - let save_dir_it = match save_dir_.make_open_path(&version_name, Default::default()) { + let save_dir_it = match save_dir_.make_open_path(&tmp_subdir, Default::default()) { Ok(d) => d, Err(err) => { Output::err_generic("Failed to open temporary directory: {}", (err.name(),)); @@ -1002,7 +1017,7 @@ impl UpgradeCommand { }; scopeguard::defer! { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); } // Zig matched `error.FileNotFound`; the bun.sys spawn path tags @@ -1033,7 +1048,7 @@ impl UpgradeCommand { }; if !result.status.is_ok() { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); let exit_code: u32 = match &result.status { Status::Exited(e) => u32::from(e.code), Status::Signaled(sig) => 128 + u32::from(*sig), @@ -1061,7 +1076,7 @@ impl UpgradeCommand { let trimmed = bun_core::trim(version_string, b" \n\r\t"); if trimmed != version_name.as_slice() { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "error: The downloaded version of Bun ({}) doesn't match the expected version ({}). Cancelled upgrade", @@ -1128,7 +1143,7 @@ impl UpgradeCommand { let target_dir_it = match sys::open_dir_absolute(target_dirname.as_bytes()) { Ok(d) => sys::Dir::from_fd(d), Err(err) => { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "error: Failed to open Bun's install directory {}", bstr::BStr::new(err.name()) @@ -1151,7 +1166,7 @@ impl UpgradeCommand { let target_stat = match sys::fstatat(target_dir.fd(), target_filename) { Ok(s) => s, Err(err) => { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "error: {} while trying to stat target {} ", bstr::BStr::new(err.name()), @@ -1164,7 +1179,7 @@ impl UpgradeCommand { let dest_stat = match sys::fstatat(save_dir.fd(), exe_z) { Ok(s) => s, Err(err) => { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "error: {} while trying to stat source {}", bstr::BStr::new(err.name()), @@ -1192,7 +1207,7 @@ impl UpgradeCommand { }) { Ok(n) => &input_buf[..n], Err(err) => { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "error: Failed to read target bun {}", bstr::BStr::new(err.name()) @@ -1212,7 +1227,7 @@ impl UpgradeCommand { ) { Ok(n) => &input_buf[..n], Err(err) => { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "error: Failed to read source bun {}", bstr::BStr::new(err.name()) @@ -1223,7 +1238,7 @@ impl UpgradeCommand { ); if target_hash == source_hash { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "Congrats! You're already on the latest canary build of Bun\n\nTo downgrade to the latest stable release, run bun upgrade --stable\n" )); @@ -1261,7 +1276,7 @@ impl UpgradeCommand { destination_executable_z, outdated_filename.as_deref().unwrap(), ) { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); Output::pretty_errorln(format_args!( "error: Failed to rename current executable {}", bstr::BStr::new(err.name()) @@ -1276,7 +1291,7 @@ impl UpgradeCommand { sys::move_file_z(save_dir.fd(), exe_z, target_dir.fd(), target_filename) { scopeguard::defer! { - let _ = save_dir_.delete_tree(&version_name); + let _ = save_dir_.delete_tree(&tmp_subdir); } #[cfg(windows)] diff --git a/src/runtime/node/net/BlockList.rs b/src/runtime/node/net/BlockList.rs index 412da4c6632..b9ec1974b79 100644 --- a/src/runtime/node/net/BlockList.rs +++ b/src/runtime/node/net/BlockList.rs @@ -370,6 +370,16 @@ impl BlockList { Ok(array) } + /// Process-wide set of `BlockList` addresses emitted by serialize. Deserialize + /// validates the embedded pointer against this set; the byte stream is untrusted. + fn serialized_registry() + -> &'static bun_threading::Guarded> { + use std::sync::OnceLock; + static REGISTRY: OnceLock>> = + OnceLock::new(); + REGISTRY.get_or_init(Default::default) + } + pub fn on_structured_clone_serialize( this: &Self, _global: &JSGlobalObject, @@ -380,6 +390,10 @@ impl BlockList { use bun_io::Write as _; let _guard = this.mutex.lock_guard(); this.ref_(); + // Register *after* the +1 ref so a registered address is always pinned alive. + Self::serialized_registry() + .lock() + .insert(std::ptr::from_ref::(this) as usize); let mut writer = StructuredCloneWriter { ctx, impl_: write_bytes, @@ -417,6 +431,14 @@ impl BlockList { // SAFETY: `r.pos <= total_length` (`read_exact` bounds-checks via `checked_add`). *ptr = unsafe { (*ptr).add(r.pos) }; + // The byte stream may be untrusted; only dereference addresses we + // ourselves emitted from serialize. + if !Self::serialized_registry().lock().contains(&int) { + return Err(global.throw(format_args!( + "BlockList.onStructuredCloneDeserialize failed" + ))); + } + let this: *mut Self = int as *mut Self; // A single SerializedScriptValue can be deserialized multiple times // (e.g. BroadcastChannel fan-out), so each wrapper must own its own ref @@ -425,9 +447,10 @@ impl BlockList { // SerializedScriptValue has no destroy hook for Bun-native tags, so that // ref is retained until a buffer-level deref exists (preferable to UAF). // SAFETY: `int` was produced by `on_structured_clone_serialize` from a - // live `*mut Self` whose ref was bumped at serialize time. Ownership of - // one ref transfers to the C++ wrapper (released via `finalize` → `deref`). - // `to_js_ptr` is the `#[bun_jsc::JsClass]`-generated `${T}__create` shim. + // live `*mut Self` whose ref was bumped at serialize time (verified by + // the registry check above). Ownership of one ref transfers to the C++ + // wrapper (released via `finalize` → `deref`). `to_js_ptr` is the + // `#[bun_jsc::JsClass]`-generated `${T}__create` shim. unsafe { (*this).ref_(); Ok(Self::to_js_ptr(this, global)) diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index e3077db6a65..e4f075a3c9d 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -3452,18 +3452,33 @@ where if let Some(filename) = self.blob.get_file_name() { let basename = bun_paths::basename(filename); if !basename.is_empty() { + // Strip control bytes and escape `"` / `\` (RFC 7230 quoted-string) + // so a crafted filename can't inject response headers. let mut filename_buf = [0u8; 1024]; + let prefix = b"filename=\""; + filename_buf[..prefix.len()].copy_from_slice(prefix); + let mut written = prefix.len(); let truncated = &basename[..basename.len().min(1024 - 32)]; - let header_value = { - let mut w = &mut filename_buf[..]; - if write!(w, "filename=\"{}\"", bstr::BStr::new(truncated)).is_ok() { - let written = 1024 - w.len(); - &filename_buf[..written] - } else { - &b""[..] + for &b in truncated { + if b < 0x20 || b == 0x7F { + continue; } - }; - resp.write_header(b"content-disposition", header_value); + let escape = b == b'"' || b == b'\\'; + let need = if escape { 2 } else { 1 }; + // Reserve one byte for the closing quote. + if written + need >= filename_buf.len() { + break; + } + if escape { + filename_buf[written] = b'\\'; + written += 1; + } + filename_buf[written] = b; + written += 1; + } + filename_buf[written] = b'"'; + written += 1; + resp.write_header(b"content-disposition", &filename_buf[..written]); } } } diff --git a/src/runtime/shell/states/Expansion.rs b/src/runtime/shell/states/Expansion.rs index 5ddaed9d4e4..831252671cd 100644 --- a/src/runtime/shell/states/Expansion.rs +++ b/src/runtime/shell/states/Expansion.rs @@ -168,12 +168,16 @@ impl Expansion { ast::Atom::Compound(c) => &c.atoms[me.word_idx as usize], }; let shell = me.base.shell(); + // When the word will be re-tokenized by the brace lexer, escape + // interpolated bytes so user-supplied `{` `,` `}` `\` stay literal. + let escape_for_braces = atom.has_brace_expansion(); let is_cmd_subst = Self::expand_simple_no_io( shell, simple, &mut me.current_out, &mut me.has_quoted_empty, true, + escape_for_braces, event_loop, command_ctx, vm_args_utf8, @@ -334,22 +338,44 @@ impl Expansion { Yield::suspended() } + /// Append `bytes`, backslash-escaping `\` `{` `,` `}` so interpolated data + /// can't introduce brace metacharacters during the second-pass brace lexer. + fn extend_brace_escaped(out: &mut Vec, bytes: &[u8]) { + for &b in bytes { + if matches!(b, b'\\' | b'{' | b',' | b'}') { + out.push(b'\\'); + } + out.push(b); + } + } + /// Spec: Expansion.zig `expandSimpleNoIO`. Appends the no-IO expansion of /// one [`ast::SimpleAtom`] to `out`. Returns `true` for `CmdSubst` so the /// caller spawns a `Script` for it. + /// + /// When `escape_for_braces`, interpolated bytes are escaped via + /// [`Self::extend_brace_escaped`] so they stay literal in the brace re-lexer. fn expand_simple_no_io( shell: &ShellExecEnv, atom: &ast::SimpleAtom, out: &mut Vec, has_quoted_empty: &mut bool, expand_tilde: bool, + escape_for_braces: bool, event_loop: EventLoopHandle, command_ctx: *mut bun_options_types::context::ContextData, vm_args_utf8: &mut Vec, ) -> bool { use crate::shell::env_str::EnvStr; + let push = |out: &mut Vec, bytes: &[u8]| { + if escape_for_braces { + Self::extend_brace_escaped(out, bytes); + } else { + out.extend_from_slice(bytes); + } + }; match atom { - ast::SimpleAtom::Text(txt) => out.extend_from_slice(txt), + ast::SimpleAtom::Text(txt) => push(out, txt), ast::SimpleAtom::QuotedEmpty => { // Spec: Expansion.zig `expandSimpleNoIO` sets // `has_quoted_empty = true` so an empty word is still pushed @@ -362,15 +388,22 @@ impl Expansion { // Spec `expandVar`: shell_env first, then export_env, else "". let key = EnvStr::init_slice(label); if let Some(v) = shell.shell_env.get(key) { - out.extend_from_slice(v.slice()); + push(out, v.slice()); v.deref(); } else if let Some(v) = shell.export_env.get(EnvStr::init_slice(label)) { - out.extend_from_slice(v.slice()); + push(out, v.slice()); v.deref(); } } ast::SimpleAtom::VarArgv(int) => { - Interpreter::append_var_argv(out, *int, event_loop, command_ctx, vm_args_utf8); + if escape_for_braces { + let pre = out.len(); + Interpreter::append_var_argv(out, *int, event_loop, command_ctx, vm_args_utf8); + let appended: Vec = out.split_off(pre); + Self::extend_brace_escaped(out, &appended); + } else { + Interpreter::append_var_argv(out, *int, event_loop, command_ctx, vm_args_utf8); + } } ast::SimpleAtom::Asterisk => out.push(b'*'), ast::SimpleAtom::DoubleAsterisk => out.extend_from_slice(b"**"), @@ -380,7 +413,7 @@ impl Expansion { ast::SimpleAtom::Tilde => { if expand_tilde { let home = shell.get_homedir(); - out.extend_from_slice(home.slice()); + push(out, home.slice()); home.deref(); } else { out.push(b'~'); diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 1faa42251b1..45445a3022a 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3784,6 +3784,27 @@ struct FormDataContext<'a> { global_this: &'a JSGlobalObject, } +/// WHATWG `multipart/form-data` escape for field names/filenames: `"` → `%22`, +/// `\r` → `%0D`, `\n` → `%0A` to prevent quoted-string/CRLF header injection. +fn escape_form_data_header_value(bytes: Vec) -> Box<[u8]> { + if !bytes + .iter() + .any(|&b| b == b'"' || b == b'\r' || b == b'\n') + { + return bytes.into_boxed_slice(); + } + let mut out = Vec::with_capacity(bytes.len() + 8); + for &b in &bytes { + match b { + b'"' => out.extend_from_slice(b"%22"), + b'\r' => out.extend_from_slice(b"%0D"), + b'\n' => out.extend_from_slice(b"%0A"), + _ => out.push(b), + } + } + out.into_boxed_slice() +} + impl FormDataContext<'_> { pub fn on_entry(&mut self, name: ZigString, entry: FormDataEntry<'_>) { if self.failed { @@ -3804,7 +3825,7 @@ impl FormDataContext<'_> { // the optional allocator. `StringJoiner::push_owned` is the Rust // equivalent; `ZigStringSlice::into_vec` moves out the buffer if owned // or copies if borrowed (matching Zig's `null`-allocator borrow case). - joiner.push_owned(name.to_slice().into_vec().into_boxed_slice()); + joiner.push_owned(escape_form_data_header_value(name.to_slice().into_vec())); match entry { FormDataEntry::String(value) => { @@ -3813,7 +3834,9 @@ impl FormDataContext<'_> { } FormDataEntry::File { blob, filename } => { joiner.push_static(b"\"; filename=\""); - joiner.push_owned(filename.to_slice().into_vec().into_boxed_slice()); + joiner.push_owned(escape_form_data_header_value( + filename.to_slice().into_vec(), + )); joiner.push_static(b"\"\r\n"); let content_type: &[u8] = if !blob.content_type_slice().is_empty() { diff --git a/src/semver/lib.rs b/src/semver/lib.rs index cd067af75e8..4cb4bc121a1 100644 --- a/src/semver/lib.rs +++ b/src/semver/lib.rs @@ -527,15 +527,9 @@ pub mod semver_string { let b = that.ptr(); let (a_off, a_len) = (a.off as usize, a.len as usize); let (b_off, b_len) = (b.off as usize, b.len as usize); - debug_assert!(a_off + a_len <= this_buf.len()); - debug_assert!(b_off + b_len <= that_buf.len()); - // SAFETY: Pointer {off,len} is constructed by `init`/`init_append` from a - // sub-slice of `buf` and is only ever projected back into the same buffer - // (Zig: `buf[ptr.off..][0..ptr.len]`, unchecked in ReleaseFast). - strings::eql( - unsafe { this_buf.get_unchecked(a_off..a_off + a_len) }, - unsafe { that_buf.get_unchecked(b_off..b_off + b_len) }, - ) + // Bounds-checked: `Pointer { off, len }` can be bit-cast from an untrusted + // binary lockfile, so an unchecked slice would be an OOB read. + strings::eql(&this_buf[a_off..a_off + a_len], &that_buf[b_off..b_off + b_len]) } } @@ -605,12 +599,9 @@ pub mod semver_string { _ => { let ptr_ = self.ptr(); let (off, len) = (ptr_.off as usize, ptr_.len as usize); - debug_assert!(off + len <= buf.len()); - // SAFETY: Pointer {off,len} is constructed by `init`/`init_append` from a - // sub-slice of `buf` and is only ever projected back into the same buffer - // (Zig: `buf[ptr.off..][0..ptr.len]`, unchecked in ReleaseFast). The two - // checked slice ops here were the dominant cost in install hot loops. - unsafe { buf.get_unchecked(off..off + len) } + // Bounds-checked: `Pointer { off, len }` can be bit-cast from an untrusted + // binary lockfile, so an unchecked slice would be an OOB read. + &buf[off..off + len] } } } diff --git a/src/shell_parser/parse.rs b/src/shell_parser/parse.rs index afe48fafc35..347c7a67328 100644 --- a/src/shell_parser/parse.rs +++ b/src/shell_parser/parse.rs @@ -3636,7 +3636,14 @@ impl<'bump, const ENCODING: StringEncoding> Lexer<'bump, ENCODING> { })); return Ok(()); } - self.append_string_to_str_pool(bunstr) + // Emit interpolated bytes as DoubleQuotedText so a user-supplied + // leading `~` is not tilde-expanded as if typed literally. + let start = self.j; + self.append_string_to_str_pool(bunstr)?; + let end = self.j; + self.tokens.push(Token::DoubleQuotedText(TextRange { start, end })); + self.word_start = self.j; + Ok(()) } fn looks_like_js_obj_ref(&mut self) -> bool { diff --git a/src/sql_jsc/mysql/MySQLConnection.rs b/src/sql_jsc/mysql/MySQLConnection.rs index 3ba36191647..31db6fd6175 100644 --- a/src/sql_jsc/mysql/MySQLConnection.rs +++ b/src/sql_jsc/mysql/MySQLConnection.rs @@ -821,7 +821,9 @@ impl MySQLConnection { Auth::caching_sha2_password::FastAuthStatus::CONTINUE_AUTH => { bun_core::scoped_log!(MySQLConnection, "continue auth"); - if self.ssl_mode == SSLMode::Disable { + // Check whether TLS actually established, not the + // configured ssl_mode (which may have fallen back). + if self.tls_status != TLSStatus::SslOk { // we are in plain TCP so we need to request the public key self.set_status(ConnectionState::AuthenticationAwaitingPk); bun_core::scoped_log!( @@ -1403,6 +1405,18 @@ impl MySQLConnection { // Can't be 0 return Err(AnyMySQLError::UnexpectedPacket); } + // `field_count` is a server-controlled length-encoded integer; cap + // it before it drives an allocation. MySQL caps tables at 4096 cols. + const MAX_RESULT_SET_COLUMNS: u64 = 0xFFFF; + if header.field_count > MAX_RESULT_SET_COLUMNS { + bun_core::scoped_log!( + MySQLConnection, + "result set header field count {} exceeds cap {}", + header.field_count, + MAX_RESULT_SET_COLUMNS + ); + return Err(AnyMySQLError::UnexpectedPacket); + } if statement.columns.len() as u64 != header.field_count { bun_core::scoped_log!( MySQLConnection, diff --git a/src/sql_jsc/postgres/PostgresRequest.rs b/src/sql_jsc/postgres/PostgresRequest.rs index 9ff3ef2c23e..4a11828e1b0 100644 --- a/src/sql_jsc/postgres/PostgresRequest.rs +++ b/src/sql_jsc/postgres/PostgresRequest.rs @@ -468,7 +468,12 @@ pub fn on_data( if matches!(connection.tls_status.get(), TlsStatus::MessageSent(_)) { connection.tls_status.set(TlsStatus::SslNotAvailable); bun_core::scoped_log!(Postgres, "Server does not support SSL"); - if connection.ssl_mode == SslMode::Require { + // `verify-ca`/`verify-full` are stronger than `require`: all three + // must refuse plaintext fallback when the server rejects SSLRequest. + if matches!( + connection.ssl_mode, + SslMode::Require | SslMode::VerifyCa | SslMode::VerifyFull + ) { connection.fail( b"Server does not support SSL", AnyPostgresError::TLSNotAvailable, diff --git a/src/sql_jsc/postgres/SASL.rs b/src/sql_jsc/postgres/SASL.rs index 82e8833149a..868d259749c 100644 --- a/src/sql_jsc/postgres/SASL.rs +++ b/src/sql_jsc/postgres/SASL.rs @@ -11,6 +11,10 @@ const SERVER_SIGNATURE_BASE64_LEN: usize = const SALTED_PASSWORD_BYTE_LEN: usize = 32; +/// Hard cap on the server-supplied SCRAM iteration count. PBKDF2 runs +/// synchronously on the JS thread, so an unbounded value pins the event loop. +const MAX_SCRAM_ITERATION_COUNT: u32 = 4096 * 1024; + pub struct SASL { pub nonce_base64_bytes: [u8; NONCE_BASE64_LEN], pub nonce_len: u8, @@ -74,6 +78,12 @@ impl SASL { use bun_boringssl_sys as boringssl; use core::ffi::c_uint; + // Reject server-controlled iteration counts that would pin the JS thread. + // Use a known AnyPostgresError variant so the error roundtrips. + if iteration_count == 0 || iteration_count > MAX_SCRAM_ITERATION_COUNT { + return Err(bun_core::err!("PBKDFD2")); + } + self.salted_password_created = true; let out = &mut self.salted_password_bytes; out.fill(0); diff --git a/src/url/lib.rs b/src/url/lib.rs index 05ab9afb33a..382eac1fe6a 100644 --- a/src/url/lib.rs +++ b/src/url/lib.rs @@ -1303,10 +1303,13 @@ impl<'a> Iterator<'a> { where 'a: 't, { - while self.visited.is_set(self.i) { + // Clamp to the fixed 2048-bit `VisitedMap` so an oversized query-string + // param list can't index past the end of `visited.masks`. + let limit = self.map.list.len().min(VisitedMap::BIT_LENGTH); + while self.i < limit && self.visited.is_set(self.i) { self.i += 1; } - if self.i >= self.map.list.len() { + if self.i >= limit { return None; } @@ -1320,7 +1323,7 @@ impl<'a> Iterator<'a> { self.visited.set(self.i); self.i += 1; - let remainder = &list[self.i..]; + let remainder = &list[self.i..limit]; let mut target_i: usize = 1; let mut current_i: usize = 0; @@ -1345,7 +1348,7 @@ impl<'a> Iterator<'a> { values: &mut target[0..target_i], }); } - if real_i + 1 >= self.map.list.len() { + if real_i + 1 >= limit { return Some(IteratorResult { name, values: &mut target[0..target_i],