diff --git a/packages/bun-types/sql.d.ts b/packages/bun-types/sql.d.ts index 59681350ffb..ebf032f4b80 100644 --- a/packages/bun-types/sql.d.ts +++ b/packages/bun-types/sql.d.ts @@ -378,6 +378,17 @@ declare module "bun" { * @default true */ prepare?: boolean | undefined; + + /** + * MySQL only. Allow the client to request the server's RSA public key + * during `caching_sha2_password` / `sha256_password` authentication when + * the connection is not protected by TLS. Disabled by default because a + * network attacker can substitute their own key and recover the + * plaintext password. Enable only for trusted local connections, or use + * TLS instead. + * @default false + */ + allowPublicKeyRetrieval?: boolean | undefined; } /** diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index b34953926df..f90f74d9f18 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -750,6 +750,12 @@ namespace uWS /* Error: invalid chars in field name */ return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_HEADER_TOKEN); } + /* RFC 9112 5.1: field-name is a non-empty token. An empty name would also + * collide with the end-of-headers sentinel and hide later headers from the + * Content-Length / Transfer-Encoding request-smuggling checks. */ + if (headers->key.length() == 0) { + 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..bbbe82a24ef 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -257,7 +257,9 @@ struct HttpResponse : public AsyncSocket { /* Note: OpenSSL can be used here to speed this up somewhat */ char secWebSocketAccept[29] = {}; - WebSocketHandshake::generate(secWebSocketKey.data(), secWebSocketAccept); + char secWebSocketKeyBuffer[24] = {}; + secWebSocketKey.copy(secWebSocketKeyBuffer, 24); + WebSocketHandshake::generate(secWebSocketKeyBuffer, secWebSocketAccept); writeStatus("101 Switching Protocols") ->writeHeader("Upgrade", "websocket") diff --git a/src/bundler/options.rs b/src/bundler/options.rs index aea186cf387..2863de3b5fe 100644 --- a/src/bundler/options.rs +++ b/src/bundler/options.rs @@ -2596,10 +2596,23 @@ 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 => { + if dir.is_empty() { + writer.write_all(b".")?; + } else { + // Sanitize leading `..` segments so `[dir]` cannot place output + // above outdir when a source resolves outside `root`. + let mut d: &[u8] = dir; + while matches!(d, [b'.', b'.', b'/' | b'\\', ..]) { + PathTemplate::write_replacing_slashes_on_windows(writer, b"_.._/")?; + d = &d[3..]; + } + PathTemplate::write_replacing_slashes_on_windows( + writer, + if d == b".." { b"_.._" } else { 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 4e4d9566389..31491d8fd7a 100644 --- a/src/css/css_parser.rs +++ b/src/css/css_parser.rs @@ -634,6 +634,8 @@ pub fn parse_until_after( result } +const MAX_NESTING_DEPTH: u32 = 512; + fn parse_nested_block( parser: &mut Parser, parsefn: impl FnOnce(&mut Parser) -> CssResult, @@ -646,6 +648,14 @@ fn parse_nested_block( ) }); + parser.input.nesting_depth += 1; + if parser.input.nesting_depth > MAX_NESTING_DEPTH { + parser.input.nesting_depth -= 1; + let err = parser.new_custom_error(ParserError::maximum_nesting_depth); + consume_until_end_of_block(block_type, &mut parser.input.tokenizer); + return Err(err); + } + let closing_delimiter = match block_type { BlockType::CurlyBracket => Delimiters::CLOSE_CURLY_BRACKET, BlockType::SquareBracket => Delimiters::CLOSE_SQUARE_BRACKET, @@ -663,6 +673,7 @@ fn parse_nested_block( } parser.stop_before = saved_stop_before; consume_until_end_of_block(block_type, &mut parser.input.tokenizer); + parser.input.nesting_depth -= 1; result } @@ -4201,6 +4212,7 @@ impl Delimiters { pub struct ParserInput<'a> { pub tokenizer: Tokenizer<'a>, pub cached_token: Option, + pub nesting_depth: u32, } impl<'a> ParserInput<'a> { @@ -4216,6 +4228,7 @@ impl<'a> ParserInput<'a> { ParserInput { tokenizer: Tokenizer::init_with_arena(code, arena), cached_token: None, + nesting_depth: 0, } } } diff --git a/src/glob/matcher.rs b/src/glob/matcher.rs index 6cc82c68214..43929729ccc 100644 --- a/src/glob/matcher.rs +++ b/src/glob/matcher.rs @@ -37,6 +37,12 @@ struct Brace { } type BraceStack = BoundedArray; +/// Upper bound on brace-branch alternatives explored per `match` call. Sequential +/// brace groups multiply (`{a,b}{c,d}` = 4 alternatives), so without a cap an +/// adversarial pattern of ten sequential 10-way groups would explore 10^10 +/// alternatives. Patterns that exceed this budget fail to match. +const BRACE_BRANCH_BUDGET: u32 = 10_000; + // PORT NOTE: made `pub` — Zig leaks this private type through `pub fn match`; Rust forbids private-in-public. #[derive(Copy, Clone, Eq, PartialEq)] pub enum MatchResult { @@ -141,7 +147,15 @@ pub fn r#match(glob: &[u8], path: &[u8]) -> MatchResult { // PORT NOTE: `BraceStack.init(0) catch unreachable` — zero-length init cannot fail. let mut brace_stack = BraceStack::default(); - let matched = glob_match_impl(&mut state, glob, 0, path, &mut brace_stack); + let mut brace_budget = BRACE_BRANCH_BUDGET; + let matched = glob_match_impl( + &mut state, + glob, + 0, + path, + &mut brace_stack, + &mut brace_budget, + ); // TODO: consider just returning a bool // return matched != negated; @@ -170,6 +184,7 @@ fn glob_match_impl( glob_start: u32, path: &[u8], brace_stack: &mut BraceStack, + brace_budget: &mut u32, ) -> bool { 'main_loop: while (state.glob_index as usize) < glob.len() || (state.path_index as usize) < path.len() @@ -348,7 +363,7 @@ fn glob_match_impl( continue 'main_loop; } } - return match_brace(state, glob, path, brace_stack); + return match_brace(state, glob, path, brace_stack, brace_budget); } b',' => { if state.brace_depth > 0 { @@ -412,7 +427,13 @@ fn glob_match_impl( true } -fn match_brace(state: &mut State, glob: &[u8], path: &[u8], brace_stack: &mut BraceStack) -> bool { +fn match_brace( + state: &mut State, + glob: &[u8], + path: &[u8], + brace_stack: &mut BraceStack, + brace_budget: &mut u32, +) -> bool { let mut brace_depth: i16 = 0; let mut in_brackets = false; @@ -441,6 +462,7 @@ fn match_brace(state: &mut State, glob: &[u8], path: &[u8], brace_stack: &mut Br open_brace_index, branch_index, brace_stack, + brace_budget, ) { return true; } @@ -457,6 +479,7 @@ fn match_brace(state: &mut State, glob: &[u8], path: &[u8], brace_stack: &mut Br open_brace_index, branch_index, brace_stack, + brace_budget, ) { return true; } @@ -485,7 +508,13 @@ fn match_brace_branch( open_brace_index: u32, branch_index: u32, brace_stack: &mut BraceStack, + brace_budget: &mut u32, ) -> bool { + if *brace_budget == 0 { + return false; + } + *brace_budget -= 1; + // exceeded brace depth let Ok(()) = brace_stack.push(Brace { open_brace_idx: open_brace_index, @@ -499,7 +528,14 @@ fn match_brace_branch( branch_state.glob_index = branch_index; branch_state.brace_depth = u8::try_from(brace_stack.len()).expect("int cast"); - let matched = glob_match_impl(&mut branch_state, glob, branch_index, path, brace_stack); + let matched = glob_match_impl( + &mut branch_state, + glob, + branch_index, + path, + brace_stack, + brace_budget, + ); let _ = brace_stack.pop(); diff --git a/src/http/lib.rs b/src/http/lib.rs index 0e230eaf8bb..504cb015e1c 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -3104,6 +3104,17 @@ impl<'a> HTTPClient<'a> { ) { Ok(r) => r, Err(picohttp::ParseResponseError::ShortRead) => { + // `MAX_HTTP_HEADER_SIZE` (default 16 KB) is the *server*/ + // request-side knob (Node `--max-http-header-size`); reusing + // it here rejects legitimate responses with large + // `Location`/`Set-Cookie` headers. The intent is to bound + // `response_message_buffer` growth, so use a generous fixed + // cap independent of that knob. + const MAX_RESPONSE_HEADER_BUFFER: usize = 1024 * 1024; + if to_read!().len() > MAX_RESPONSE_HEADER_BUFFER { + self.close_and_fail::(err!(ResponseHeadersTooLarge), socket); + return; + } self.handle_short_read::(incoming_data, socket, needs_move); return; } diff --git a/src/http_jsc/websocket_client.rs b/src/http_jsc/websocket_client.rs index 55590fbf730..d4b631519cf 100644 --- a/src/http_jsc/websocket_client.rs +++ b/src/http_jsc/websocket_client.rs @@ -52,6 +52,11 @@ pub type Socket = NewSocketHandler; const STACK_FRAME_SIZE: usize = 1024; /// Minimum message size to compress (RFC 7692 recommendation) const MIN_COMPRESS_SIZE: usize = 860; +/// Maximum buffered inbound message size (128 MB). A server that declares a +/// larger frame, or whose continuation fragments accumulate past this, fails +/// the connection with close code 1009 instead of growing `receive_buffer` +/// without bound. +const MAX_RECEIVE_MESSAGE_LENGTH: usize = 128 * 1024 * 1024; #[derive(bun_ptr::CellRefCounted)] #[ref_count(destroy = Self::deinit)] pub struct WebSocket { @@ -946,6 +951,16 @@ impl WebSocket { } } ReceiveState::NeedBody => { + if self + .receive_buffer + .readable_length() + .saturating_add(receive_body_remain) + > MAX_RECEIVE_MESSAGE_LENGTH + { + self.terminate(ErrorCode::MessageTooBig); + terminated = true; + break; + } let to_consume = receive_body_remain.min(data.len()); let consumed = self.consume( diff --git a/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs b/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs index 48d8e5dc12c..a4b06e31f69 100644 --- a/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs +++ b/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs @@ -1332,7 +1332,7 @@ impl HTTPClient { header.name(), b"Sec-WebSocket-Version", ) { - if !strings::eql_comptime_ignore_len(header.value(), b"13") { + if !strings::eql_comptime(header.value(), b"13") { // SAFETY: no `&mut Self` is live across this call. unsafe { Self::terminate(this, ErrorCode::InvalidWebsocketVersion) }; return; diff --git a/src/http_types/URLPath.rs b/src/http_types/URLPath.rs index 127f5354559..4470758f077 100644 --- a/src/http_types/URLPath.rs +++ b/src/http_types/URLPath.rs @@ -98,13 +98,13 @@ pub fn parse(possibly_encoded_pathname_: &[u8]) -> Result= 0 { let c = decoded_pathname[usize::try_from(i).expect("int cast")]; diff --git a/src/install/bin.rs b/src/install/bin.rs index d9cfcfc41b5..3ee27a7d7e1 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -740,11 +740,18 @@ pub type Context = PriorityQueueContext; // https://github.com/npm/npm-normalize-package-bin/blob/574e6d7cd21b2f3dee28a216ec2053c2551f7af9/lib/index.js#L38 pub fn normalized_bin_name(name: &[u8]) -> &[u8] { - if let Some(i) = name + let name = match name .iter() .rposition(|&b| b == b'/' || b == b'\\' || b == b':') { - return &name[i + 1..]; + Some(i) => &name[i + 1..], + None => name, + }; + + // npm's `join('/', key).slice(1)` collapses `.`/`..` to empty; do the same + // so the `.bin/` destination cannot resolve outside `.bin/`. + if name == b"." || name == b".." { + return b""; } name diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index bd9784ba9d2..38c1679c852 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -2483,6 +2483,19 @@ pub fn parse_into_binary_lockfile( }; pkg.meta.integrity = Integrity::parse(integrity_str); + if !integrity_str.is_empty() && !pkg.meta.integrity.tag.is_supported() { + // Surface — don't fail — for npm parity (`npm install` + // proceeds on a malformed lockfile integrity, treating + // it as absent). The download path still applies any + // registry-supplied integrity, so this only loses the + // *lockfile* pin. + log.add_warning( + Some(source), + integrity_expr.loc, + b"Unsupported or malformed integrity hash; ignoring", + ); + pkg.meta.integrity = Integrity::default(); + } } ResolutionTag::LocalTarball | ResolutionTag::RemoteTarball => { // integrity is optional for tarball deps (backward compat) @@ -2490,6 +2503,14 @@ pub fn parse_into_binary_lockfile( let integrity_expr = pkg_info.at(i); if let Some(integrity_str) = integrity_expr.as_utf8_string_literal() { pkg.meta.integrity = Integrity::parse(integrity_str); + if !integrity_str.is_empty() && !pkg.meta.integrity.tag.is_supported() { + log.add_warning( + Some(source), + integrity_expr.loc, + b"Unsupported or malformed integrity hash; ignoring", + ); + pkg.meta.integrity = Integrity::default(); + } } } } @@ -2508,6 +2529,11 @@ pub fn parse_into_binary_lockfile( return Err(ParseError::InvalidPackageInfo); }; + if !crate::repository::is_safe_resolved_tag(bun_tag_str) { + log.add_error(Some(source), bun_tag.loc, b"Invalid git dependency tag"); + return Err(ParseError::InvalidPackageInfo); + } + let resolved = sbuf!(lockfile).append(bun_tag_str)?; if tag == ResolutionTag::Git { res.git_mut().resolved = resolved; @@ -2520,6 +2546,14 @@ pub fn parse_into_binary_lockfile( let integrity_expr = pkg_info.at(i); if let Some(integrity_str) = integrity_expr.as_utf8_string_literal() { pkg.meta.integrity = Integrity::parse(integrity_str); + if !integrity_str.is_empty() && !pkg.meta.integrity.tag.is_supported() { + log.add_warning( + Some(source), + integrity_expr.loc, + b"Unsupported or malformed integrity hash; ignoring", + ); + pkg.meta.integrity = Integrity::default(); + } } } } diff --git a/src/install/repository.rs b/src/install/repository.rs index eb4f124eeb1..f7de6a1a780 100644 --- a/src/install/repository.rs +++ b/src/install/repository.rs @@ -301,6 +301,22 @@ pub fn host_tld(host: &[u8]) -> Option<&'static [u8]> { } } +/// `resolved` is the `.bun-tag` value persisted to the lockfile (a commit SHA for +/// `git`, or `--` for `github`). It is concatenated into a cache +/// directory name and passed to `git checkout`, so it must be a single safe path +/// component: no separators, no NUL, and no leading `-` that git would parse as an +/// option. +pub fn is_safe_resolved_tag(resolved: &[u8]) -> bool { + !resolved.is_empty() + && resolved.len() <= 256 + && resolved[0] != b'-' + && resolved != b"." + && resolved != b".." + && resolved + .iter() + .all(|&b| b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_' | b'.')) +} + /// Install-tier `Repository` behaviour (parsing, formatting, git CLI exec, /// download/checkout). Data struct + buffer-relative `order`/`count`/`clone`/ /// `eql` are inherent on [`Repository`] (defined in `bun_install_types`). @@ -826,6 +842,20 @@ impl RepositoryExt for Repository { // `cache_dir`/`repo_dir` are borrowed views; only `package_dir` is owned. bun_analytics::features::git_dependencies .fetch_add(1, std::sync::atomic::Ordering::Relaxed); + + if !is_safe_resolved_tag(resolved) { + log.add_error_fmt( + None, + bun_ast::Loc::EMPTY, + format_args!( + "invalid git commit \"{}\" for \"{}\"", + BStr::new(resolved), + BStr::new(name) + ), + ); + return Err(err!("InstallFailed")); + } + let folder_name_buf = TlBufs::folder_name_buf(); let folder_name = crate::package_manager_real::cached_git_folder_name_print( &mut folder_name_buf[..], @@ -884,6 +914,8 @@ impl RepositoryExt for Repository { if let Err(err) = exec( env, + // `is_safe_resolved_tag` above rejects a leading `-`, so + // `resolved` cannot be parsed as a git option. &[b"git", b"-C", folder, b"checkout", b"--quiet", resolved], ) { log.add_error_fmt( diff --git a/src/js/bun/sql.ts b/src/js/bun/sql.ts index dc436d367fe..49bd9ae424f 100644 --- a/src/js/bun/sql.ts +++ b/src/js/bun/sql.ts @@ -740,9 +740,11 @@ const SQL: typeof Bun.SQL = function SQL( if ($isCallable(name)) { savepoint_callback = name as unknown as TransactionCallback; name = ""; + } else if (name) { + name = String(name).replace(/[^a-zA-Z0-9_]/g, "_"); } if (!$isCallable(savepoint_callback)) { - throw $ERR_INVALID_ARG_VALUE("fn", callback, "must be a function"); + throw $ERR_INVALID_ARG_VALUE("fn", savepoint_callback, "must be a function"); } // matchs the format of the savepoint name in postgres package const save_point_name = `s${savepoints++}${name ? `_${name}` : ""}`; diff --git a/src/js/internal/debugger.ts b/src/js/internal/debugger.ts index dab0013d340..4b400473692 100644 --- a/src/js/internal/debugger.ts +++ b/src/js/internal/debugger.ts @@ -592,7 +592,7 @@ function parseUrl(input: string): URL { } function randomId() { - return Math.random().toString(36).slice(2); + return crypto.randomUUID(); } const { enableANSIColors } = Bun; diff --git a/src/js/internal/sql/mysql.ts b/src/js/internal/sql/mysql.ts index aa66d74655e..2df129b99da 100644 --- a/src/js/internal/sql/mysql.ts +++ b/src/js/internal/sql/mysql.ts @@ -102,6 +102,7 @@ export interface MySQLDotZig { connectionTimeout: number, maxLifetime: number, useUnnamedPreparedStatements: boolean, + allowPublicKeyRetrieval: boolean, ) => $ZigGeneratedClasses.MySQLConnection; createQuery: ( sql: string, @@ -251,6 +252,7 @@ class PooledMySQLConnection { maxLifetime = 0, prepare = true, path, + allowPublicKeyRetrieval = false, } = options; let password: Bun.MaybePromise | string | undefined | (() => Bun.MaybePromise) = options.password; @@ -284,6 +286,7 @@ class PooledMySQLConnection { connectionTimeout, maxLifetime, !prepare, + !!allowPublicKeyRetrieval, ); } catch (e) { process.nextTick(closeNT, onClose, e); diff --git a/src/js/internal/sql/postgres.ts b/src/js/internal/sql/postgres.ts index efc3499b01d..d9271ffa4af 100644 --- a/src/js/internal/sql/postgres.ts +++ b/src/js/internal/sql/postgres.ts @@ -210,7 +210,14 @@ function getArrayType(typeNameOrID: number | ArrayType | undefined = undefined): return getPostgresArrayType(typeNameOrID as number) ?? "JSON"; } if (typeOfType === "string") { - return (typeNameOrID as string)?.toUpperCase(); + const type = (typeNameOrID as string).toUpperCase(); + // Allow `NUMERIC(10,2)`, `CHARACTER VARYING(255)`, `MYSCHEMA.MY_ENUM` + // — alnum, underscore, space, dot, comma, parens. The only goal is to + // refuse anything that could break out of the `$N::${type}[]` cast. + if (!/^[A-Z_][A-Z0-9_ .,()]*$/.test(type)) { + throw $ERR_INVALID_ARG_VALUE("type", typeNameOrID, "must be a valid PostgreSQL type name"); + } + return type; } // default to JSON so we accept most of the types return "JSON"; diff --git a/src/js/internal/sql/shared.ts b/src/js/internal/sql/shared.ts index fc484b00c77..d55ca62ad18 100644 --- a/src/js/internal/sql/shared.ts +++ b/src/js/internal/sql/shared.ts @@ -865,6 +865,7 @@ function parseOptions( database, tls, prepare, + allowPublicKeyRetrieval: options.allowPublicKeyRetrieval === true, bigint, sslMode, query, diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index 4f6e056aade..e2e9fb0a904 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -132,6 +132,7 @@ if ($debug) { */ function spawn(file, args, options) { options = normalizeSpawnArguments(file, args, options); + if (options.windowsBatchFileError) throw options.windowsBatchFileError; validateTimeout(options.timeout); validateAbortSignal(options.signal, "options.signal"); const killSignal = sanitizeKillSignal(options.killSignal); @@ -491,6 +492,26 @@ function spawnSync(file, args, options) { ...normalizeSpawnArguments(file, args, options), }; + if (options.windowsBatchFileError) { + const error = new SystemError( + `spawnSync ${options.file} EINVAL`, + options.file, + "spawnSync " + options.file, + -4071, + "EINVAL", + ); + error.spawnargs = ArrayPrototypeSlice.$call(options.args, 1); + return { + signal: null, + status: null, + output: [null, null, null], + pid: 0, + stdout: null, + stderr: null, + error, + }; + } + const maxBuffer = options.maxBuffer; const encoding = options.encoding; @@ -931,6 +952,7 @@ function normalizeSpawnArguments(file, args, options) { validateBoolean(windowsVerbatimArguments, "options.windowsVerbatimArguments"); } + let windowsBatchFileError: Error | undefined; // Handle shell if (options.shell) { validateArgumentNullCheck(options.shell, "options.shell"); @@ -952,6 +974,13 @@ function normalizeSpawnArguments(file, args, options) { else file = "/bin/sh"; args = ["-c", command]; } + } else if (process.platform === "win32" && /\.(?:cmd|bat)[\s.]*$/i.exec(file) !== null) { + // CreateProcess routes .bat/.cmd through cmd.exe, which re-parses argv + // with shell metacharacter rules. Reject unless the caller explicitly + // opted into shell semantics. Surface via the returned options so each + // caller can deliver the error in its API-appropriate shape (synchronous + // throw for spawn(), `result.error` for spawnSync()). + windowsBatchFileError = new SystemError(`spawn ${file} EINVAL`, file, "spawn", -4071, "EINVAL"); } // Handle argv0 @@ -1008,6 +1037,7 @@ function normalizeSpawnArguments(file, args, options) { windowsHide: !!options.windowsHide, windowsVerbatimArguments: !!windowsVerbatimArguments, argv0: options.argv0, + windowsBatchFileError, }; } diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index 91642716488..4ebb8eeec9f 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -2385,12 +2385,17 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_writeBody(JSC::JSGlobalObje } if (lengthValue.isUndefined() && offsetValue.isString()) { encodingValue = offsetValue; - offset = 0; - length = castedThis->byteLength(); auto* str = stringValue.toString(lexicalGlobalObject); + RETURN_IF_EXCEPTION(scope, {}); auto encoding = parseEncoding(scope, lexicalGlobalObject, encodingValue, false); RETURN_IF_EXCEPTION(scope, {}); + if (castedThis->isDetached()) [[unlikely]] { + throwTypeError(lexicalGlobalObject, scope, "ArrayBufferView is detached"_s); + return {}; + } + offset = 0; + length = castedThis->byteLength(); RELEASE_AND_RETURN(scope, writeToBuffer(lexicalGlobalObject, castedThis, str, offset, length, encoding)); } else { length = castedThis->byteLength(); @@ -2423,6 +2428,16 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_writeBody(JSC::JSGlobalObje auto encoding = parseEncoding(scope, lexicalGlobalObject, encodingValue, false); RETURN_IF_EXCEPTION(scope, {}); + if (castedThis->isDetached()) [[unlikely]] { + throwTypeError(lexicalGlobalObject, scope, "ArrayBufferView is detached"_s); + return {}; + } + uint32_t currentByteLength = castedThis->byteLength(); + if (offset >= currentByteLength) + RELEASE_AND_RETURN(scope, JSValue::encode(jsNumber(0))); + uint32_t currentRemaining = currentByteLength - offset; + if (length > currentRemaining) length = currentRemaining; + RELEASE_AND_RETURN(scope, writeToBuffer(lexicalGlobalObject, castedThis, str, offset, length, encoding)); } diff --git a/src/jsc/bindings/NodeHTTP.cpp b/src/jsc/bindings/NodeHTTP.cpp index 26958180648..37959b9a913 100644 --- a/src/jsc/bindings/NodeHTTP.cpp +++ b/src/jsc/bindings/NodeHTTP.cpp @@ -347,10 +347,12 @@ static EncodedJSValue assignHeadersFromUWebSockets(uWS::HttpRequest* request, JS HTTPHeaderName name; WTF::String nameString; WTF::String lowercasedNameString; + bool isSetCookie = false; if (WebCore::findHTTPHeaderName(nameView, name)) { nameString = WTF::httpHeaderNameStringImpl(name); lowercasedNameString = nameString; + isSetCookie = name == WebCore::HTTPHeaderName::SetCookie; } else { nameString = nameView.toString(); lowercasedNameString = nameString.convertToASCIILowercase(); @@ -358,7 +360,7 @@ static EncodedJSValue assignHeadersFromUWebSockets(uWS::HttpRequest* request, JS JSString* jsValue = jsString(vm, value); - if (name == WebCore::HTTPHeaderName::SetCookie) { + if (isSetCookie) { if (!setCookiesHeaderArray) { setCookiesHeaderArray = constructEmptyArray(globalObject, nullptr); RETURN_IF_EXCEPTION(scope, {}); diff --git a/src/jsc/bindings/node/http/JSConnectionsList.cpp b/src/jsc/bindings/node/http/JSConnectionsList.cpp index dd8175344bc..0a2c017f619 100644 --- a/src/jsc/bindings/node/http/JSConnectionsList.cpp +++ b/src/jsc/bindings/node/http/JSConnectionsList.cpp @@ -139,12 +139,11 @@ JSArray* JSConnectionsList::expired(JSGlobalObject* globalObject, uint64_t heade auto iter = JSSetIterator::create(vm, globalObject->setIteratorStructure(), active, IterationKind::Keys); RETURN_IF_EXCEPTION(scope, nullptr); - JSValue item = iter->next(vm); + JSValue item; size_t i = 0; - while (!item.isEmpty()) { + while (iter->next(globalObject, item)) { JSHTTPParser* parser = dynamicDowncast(item); if (!parser) { - item = iter->next(vm); continue; } diff --git a/src/jsc/bindings/sqlite/JSSQLStatement.cpp b/src/jsc/bindings/sqlite/JSSQLStatement.cpp index 2b71e5266f8..4343288aefe 100644 --- a/src/jsc/bindings/sqlite/JSSQLStatement.cpp +++ b/src/jsc/bindings/sqlite/JSSQLStatement.cpp @@ -1984,7 +1984,7 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementSetPrototypeFunction, (JSGlobalObject * l { auto& vm = JSC::getVM(lexicalGlobalObject); auto scope = DECLARE_THROW_SCOPE(vm); - auto* castedThis = uncheckedDowncast(callFrame->thisValue()); + auto* castedThis = dynamicDowncast(callFrame->thisValue()); CHECK_THIS diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index 217482109b1..631226598d7 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/src/jsc/bindings/webcore/SerializedScriptValue.cpp @@ -2476,9 +2476,9 @@ class CloneSerializer : public CloneBase { write(key.secondPrimeInfo().factorCRTExponent); write(key.secondPrimeInfo().factorCRTCoefficient); for (unsigned i = 2; i < primeCount; ++i) { - write(key.otherPrimeInfos()[i].primeFactor); - write(key.otherPrimeInfos()[i].factorCRTExponent); - write(key.otherPrimeInfos()[i].factorCRTCoefficient); + write(key.otherPrimeInfos()[i - 2].primeFactor); + write(key.otherPrimeInfos()[i - 2].factorCRTExponent); + write(key.otherPrimeInfos()[i - 2].factorCRTCoefficient); } } @@ -3983,7 +3983,7 @@ class CloneDeserializer : public CloneBase { bool read(BIO** bio, uint64_t length) { - if (m_ptr + length > m_end) + if (static_cast(m_end - m_ptr) < length) return false; *bio = BIO_new_mem_buf(m_ptr, length); if (!*bio) @@ -4082,11 +4082,11 @@ class CloneDeserializer : public CloneBase { if (!read(secondPrimeInfo.factorCRTCoefficient)) return false; for (unsigned i = 2; i < primeCount; ++i) { - if (!read(otherPrimeInfos[i].primeFactor)) + if (!read(otherPrimeInfos[i - 2].primeFactor)) return false; - if (!read(otherPrimeInfos[i].factorCRTExponent)) + if (!read(otherPrimeInfos[i - 2].factorCRTExponent)) return false; - if (!read(otherPrimeInfos[i].factorCRTCoefficient)) + if (!read(otherPrimeInfos[i - 2].factorCRTCoefficient)) return false; } diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp index de29f6f753a..5ffa9d3b537 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp @@ -110,7 +110,7 @@ RefPtr CryptoKeyOKP::importSpki(CryptoAlgorithmIdentifier identifi }; // Read BIT STRING - if (keyData.size() < index + 1) + if (keyData.size() < index + 2) return nullptr; if (keyData[index++] != 3) return nullptr; @@ -217,7 +217,7 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif // Read length index += bytesUsedToEncodedLength(keyData[index]); - if (keyData.size() < index + 1) + if (keyData.size() < index + 5) return nullptr; // Read OID @@ -236,14 +236,14 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif }; // Read OCTET STRING - if (keyData.size() < index + 1) + if (keyData.size() < index + 2) return nullptr; if (keyData[index++] != 4) return nullptr; index += bytesUsedToEncodedLength(keyData[index]); - if (keyData.size() < index + 1) + if (keyData.size() < index + 2) return nullptr; // Read OCTET STRING diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index b2a719a3c79..3dbe9655f45 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -1309,9 +1309,9 @@ impl Drop for BufferReadStream { /// Returns true if the symlink is safe (target stays within extraction dir), /// false if it would escape (e.g., via ../ traversal or absolute path). /// -/// The check works by resolving the symlink target relative to the symlink's -/// directory location using a fake root, then checking if the result stays -/// within that fake root. +/// The check works by normalizing `symlink_dir/link_target` as a relative +/// path with leading `..` preserved; the target is unsafe if the result +/// climbs above the extraction root. #[cfg(unix)] fn is_symlink_target_safe( symlink_path: &[u8], @@ -1327,20 +1327,35 @@ fn is_symlink_target_safe( // Get the directory containing the symlink let symlink_dir = bun_paths::dirname_simple(symlink_path); - // Use a fake root to resolve the path and check if it escapes - let fake_root: &[u8] = b"/packages/"; - let join_buf: &mut PathBuffer = symlink_join_buf.get_or_insert_with(bun_paths::path_buffer_pool::get); - let resolved = bun_paths::resolve_path::join_abs_string_buf::( - fake_root, - &mut join_buf[..], - &[symlink_dir, link_target_bytes], + // Normalize symlink_dir/link_target as a relative path. An absolute fake + // root cannot be used here: POSIX normalization clamps excess `..` at `/`, + // so a target like `../../../packages/x` would normalize back under any + // fake root that happens to match a real ancestor directory name. + if symlink_dir.len() + 1 + link_target_bytes.len() >= join_buf.len() { + return false; + } + let mut written = 0usize; + if !symlink_dir.is_empty() { + join_buf[..symlink_dir.len()].copy_from_slice(symlink_dir); + written = symlink_dir.len(); + join_buf[written] = b'/'; + written += 1; + } + join_buf[written..written + link_target_bytes.len()].copy_from_slice(link_target_bytes); + written += link_target_bytes.len(); + + let mut norm_buf = bun_paths::path_buffer_pool::get(); + let resolved = bun_paths::resolve_path::normalize_string_generic_t::( + &join_buf[..written], + &mut norm_buf[..], + b'/', + |c| c == b'/', ); - // If the resolved path doesn't start with our fake root, it escaped - resolved.starts_with(fake_root) + !(strings::eql(resolved, b"..") || strings::has_prefix_comptime(resolved, b"../")) } /// Returns true if any leading component of `path` (including the full path) @@ -1358,7 +1373,14 @@ fn path_traverses_created_symlink(path: &[u8], created_symlinks: &[Vec]) -> while end <= path.len() { if end == path.len() || path[end] == sep { let prefix = &path[..end]; - if !prefix.is_empty() && created_symlinks.iter().any(|s| s.as_slice() == prefix) { + // Compare case-insensitively: on case-insensitive filesystems (APFS + // default on macOS) an entry `LINK/x` traverses a symlink stored as + // `link`, but a byte-exact compare would miss it. + if !prefix.is_empty() + && created_symlinks + .iter() + .any(|s| strings::eql_case_insensitive_ascii_check_length(s, prefix)) + { return true; } } diff --git a/src/patch/lib.rs b/src/patch/lib.rs index c30745bd8ce..2e93d1eb4fe 100644 --- a/src/patch/lib.rs +++ b/src/patch/lib.rs @@ -106,6 +106,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()); @@ -210,12 +215,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 95739090e41..6872081108c 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -3108,28 +3108,50 @@ fn find_invalid_segment(path_: &[u8]) -> Option<&[u8]> { path = b""; } - match segment.len() { - 1 => { - if segment == b"." { - return Some(segment); - } - } - 2 => { - if segment == b".." { - return Some(segment); + if is_invalid_segment(segment) { + return Some(segment); + } + } + + None +} + +// Node's PACKAGE_TARGET_RESOLVE rejects ".", "..", and "node_modules" segments +// case-insensitively and including percent-encoded variants. Decode the segment +// before comparing so spellings like "%2e%2e" or ".%2E" cannot survive the check +// only to be decoded into ".." by `finalize`. +fn is_invalid_segment(segment: &[u8]) -> bool { + let mut decoded = [0u8; 12]; + let mut len = 0usize; + let mut i = 0usize; + while i < segment.len() { + let b = segment[i]; + let c = if b == b'%' && i + 2 < segment.len() { + match ( + bun_core::fmt::hex_digit_value(segment[i + 1]), + bun_core::fmt::hex_digit_value(segment[i + 2]), + ) { + (Some(hi), Some(lo)) => { + i += 3; + (hi << 4) | lo } - } - 12 => { - // "node_modules".len - if segment == b"node_modules" { - return Some(segment); + _ => { + i += 1; + b } } - _ => {} + } else { + i += 1; + b + }; + if len == decoded.len() { + return false; } + decoded[len] = c.to_ascii_lowercase(); + len += 1; } - - None + let d = &decoded[..len]; + d == b"." || d == b".." || d == b"node_modules" } // ported from: src/resolver/package_json.zig diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 2faa3c30ffd..3166f2891b1 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -1406,6 +1406,8 @@ pub struct Stream { wait_for_trailers: bool, end_after_headers: bool, is_waiting_more_headers: bool, + header_block_size: usize, + header_block_count: usize, padding: Option, padding_strategy: PaddingStrategy, rst_code: u32, @@ -1958,6 +1960,8 @@ impl Stream { wait_for_trailers: false, end_after_headers: false, is_waiting_more_headers: false, + header_block_size: 0, + header_block_count: 0, padding: None, padding_strategy, rst_code: 0, @@ -3194,7 +3198,7 @@ impl H2FrameParser { if let Some(s) = stream { // SAFETY: s is *mut Stream from self.streams; valid while the map entry exists unsafe { (*s).remote_window_size += window_size_increment.uint31() as u64 }; - } else { + } else if frame.stream_identifier == 0 { self.remote_window_size .set(self.remote_window_size.get() + window_size_increment.uint31() as u64); } @@ -3235,9 +3239,6 @@ impl H2FrameParser { headers.ensure_still_alive(); let mut sensitive_headers: JSValue = JSValue::UNDEFINED; - let mut count: usize = 0; - // RFC 7540 Section 6.5.2: Track cumulative header list size - let mut header_list_size: usize = 0; loop { let header = match self.decode(&payload[offset..]) { @@ -3264,10 +3265,11 @@ impl H2FrameParser { // RFC 7540 Section 6.5.2: Calculate header list size // Size = name length + value length + HPACK entry overhead per header - header_list_size += header.name.len() + header.value.len() + HPACK_ENTRY_OVERHEAD; + stream.header_block_size += + header.name.len() + header.value.len() + HPACK_ENTRY_OVERHEAD; // Check against maxHeaderListSize setting - if header_list_size > self.local_settings.get().max_header_list_size as usize { + if stream.header_block_size > self.local_settings.get().max_header_list_size as usize { self.rejected_streams.set(self.rejected_streams.get() + 1); if self.max_rejected_streams.get() <= self.rejected_streams.get() { self.send_go_away( @@ -3283,8 +3285,8 @@ impl H2FrameParser { return Ok(self.streams.get().get(&stream_id).copied()); } - count += 1; - if (self.max_header_list_pairs.get() as usize) < count { + stream.header_block_count += 1; + if (self.max_header_list_pairs.get() as usize) < stream.header_block_count { self.rejected_streams.set(self.rejected_streams.get() + 1); if self.max_rejected_streams.get() <= self.rejected_streams.get() { self.send_go_away( @@ -3386,12 +3388,7 @@ impl H2FrameParser { // SAFETY: stream_ptr is a *mut Stream stored in self.streams (heap::alloc); valid for the lifetime of the entry, exclusive access reshaped for borrowck let mut stream = unsafe { &mut *stream_ptr }; - let settings = self - .remote_settings - .get() - .unwrap_or_else(|| self.local_settings.get()); - - let max_frame_size = settings.max_frame_size; + let max_frame_size = self.local_settings.get().max_frame_size; if frame.length > max_frame_size { bun_output::scoped_log!( H2FrameParser, @@ -3548,7 +3545,7 @@ impl H2FrameParser { &self, frame: FrameHeader, data: &[u8], - stream_: Option<*mut Stream>, + _stream_: Option<*mut Stream>, ) -> usize { bun_output::scoped_log!( H2FrameParser, @@ -3556,7 +3553,7 @@ impl H2FrameParser { frame.stream_identifier, BStr::new(data) ); - if stream_.is_some() { + if frame.stream_identifier != 0 { self.send_go_away( frame.stream_identifier, ErrorCode::PROTOCOL_ERROR, @@ -3566,12 +3563,7 @@ impl H2FrameParser { ); return data.len(); } - let settings = self - .remote_settings - .get() - .unwrap_or_else(|| self.local_settings.get()); - - if frame.length < 8 || frame.length > settings.max_frame_size { + if frame.length < 8 || frame.length > self.local_settings.get().max_frame_size { self.send_go_away( frame.stream_identifier, ErrorCode::FRAME_SIZE_ERROR, @@ -3838,9 +3830,9 @@ impl H2FrameParser { &self, frame: FrameHeader, data: &[u8], - stream_: Option<*mut Stream>, + _stream_: Option<*mut Stream>, ) -> usize { - if stream_.is_some() { + if frame.stream_identifier != 0 { self.send_go_away( frame.stream_identifier, ErrorCode::PROTOCOL_ERROR, @@ -3874,6 +3866,16 @@ impl H2FrameParser { // if is not ACK send response if is_not_ack { + if self.get_session_memory_usage() > self.max_session_memory.get() as usize { + self.send_go_away( + frame.stream_identifier, + ErrorCode::ENHANCE_YOUR_CALM, + b"ENHANCE_YOUR_CALM", + self.last_stream_id.get(), + true, + ); + return end; + } self.send_ping(true, &payload_owned); } else { self.out_standing_pings @@ -3903,29 +3905,37 @@ impl H2FrameParser { data: &[u8], stream_: Option<*mut Stream>, ) -> usize { - let Some(stream_ptr) = stream_ else { + if frame.length as usize != StreamPriority::BYTE_SIZE { self.send_go_away( frame.stream_identifier, - ErrorCode::PROTOCOL_ERROR, - b"Priority frame on connection stream", + ErrorCode::FRAME_SIZE_ERROR, + b"invalid Priority frame size", self.last_stream_id.get(), true, ); return data.len(); - }; - // SAFETY: stream_ptr is a *mut Stream stored in self.streams (heap::alloc); valid for the lifetime of the entry, exclusive access reshaped for borrowck - let stream = unsafe { &mut *stream_ptr }; - - if frame.length as usize != StreamPriority::BYTE_SIZE { + } + let Some(stream_ptr) = stream_ else { + if frame.stream_identifier != 0 { + // PRIORITY on an idle/closed stream is permitted (RFC 9113 §5.3.4); ignore it. + if let Some(content) = self.handle_incomming_payload(data, frame.stream_identifier) + { + self.read_buffer.with_mut(|rb| rb.reset()); + return content.end; + } + return data.len(); + } self.send_go_away( frame.stream_identifier, - ErrorCode::FRAME_SIZE_ERROR, - b"invalid Priority frame size", + ErrorCode::PROTOCOL_ERROR, + b"Priority frame on connection stream", self.last_stream_id.get(), true, ); return data.len(); - } + }; + // SAFETY: stream_ptr is a *mut Stream stored in self.streams (heap::alloc); valid for the lifetime of the entry, exclusive access reshaped for borrowck + let stream = unsafe { &mut *stream_ptr }; if let Some(content) = self.handle_incomming_payload(data, frame.stream_identifier) { let payload = content.data(); @@ -3986,35 +3996,29 @@ impl H2FrameParser { ); return Ok(data.len()); } + if frame.length > self.local_settings.get().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 { - stream.is_waiting_more_headers = false; - if frame.flags & HeadersFrameFlags::END_STREAM as u8 != 0 { - let identifier = stream.get_identifier(); - identifier.ensure_still_alive(); - if stream.state == StreamState::HALF_CLOSED_REMOTE { - // no more continuation headers we can call it closed - stream.state = StreamState::CLOSED; - stream.free_resources::(self); - } else { - stream.state = StreamState::HALF_CLOSED_LOCAL; - } - self.dispatch_with_extra( - JSH2FrameParser::Gc::onStreamEnd, - identifier, - JSValue::js_number(stream.state as u8 as f64), - ); - } - } + // END_STREAM (end_after_headers) was already finalized by + // handle_headers_frame; only track END_HEADERS here. + stream.is_waiting_more_headers = + frame.flags & HeadersFrameFlags::END_HEADERS as u8 == 0; return Ok(end); } @@ -4050,11 +4054,7 @@ impl H2FrameParser { // SAFETY: stream_ptr is a *mut Stream stored in self.streams (heap::alloc); valid for the lifetime of the entry, exclusive access reshaped for borrowck let mut stream = unsafe { &mut *stream_ptr }; - let settings = self - .remote_settings - .get() - .unwrap_or_else(|| self.local_settings.get()); - if frame.length > settings.max_frame_size { + if frame.length > self.local_settings.get().max_frame_size { self.send_go_away( frame.stream_identifier, ErrorCode::FRAME_SIZE_ERROR, @@ -4124,6 +4124,8 @@ impl H2FrameParser { } let end = payload.len() - padding; stream.end_after_headers = frame.flags & HeadersFrameFlags::END_STREAM as u8 != 0; + stream.header_block_size = 0; + stream.header_block_count = 0; stream = match self.decode_header_block(&payload[offset..end], stream, frame.flags)? { // SAFETY: s is *mut Stream from self.streams (heap::alloc); valid while the map entry exists Some(s) => unsafe { &mut *s }, @@ -4423,6 +4425,26 @@ impl H2FrameParser { Some(stream) } + /// Stream lookup for inbound frames. Only a HEADERS frame may allocate new + /// stream state (RFC 9113 §5.1); any other frame type referencing an + /// unknown stream id is treated as idle/closed and does not allocate. + fn lookup_inbound_stream(&self, stream_identifier: u32, frame_type: u8) -> Option<*mut Stream> { + if stream_identifier == 0 { + return None; + } + if let Some(stream) = self.streams.get().get(&stream_identifier).copied() { + return Some(stream); + } + if frame_type != FrameType::HTTP_FRAME_HEADERS as u8 || !self.is_server.get() { + return None; + } + // Client-initiated streams must use odd identifiers (RFC 9113 §5.1.1). + if stream_identifier & 1 == 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 { @@ -4465,7 +4487,7 @@ impl H2FrameParser { header.stream_identifier ); - let stream = self.handle_received_stream_id(header.stream_identifier); + let stream = self.lookup_inbound_stream(header.stream_identifier, header.type_); return self.dispatch_frame(header, bytes, stream, 0); } @@ -4512,7 +4534,7 @@ impl H2FrameParser { header.flags, header.stream_identifier ); - let stream = self.handle_received_stream_id(header.stream_identifier); + let stream = self.lookup_inbound_stream(header.stream_identifier, header.type_); return self.dispatch_frame(header, &bytes[needed..], stream, needed); } @@ -4547,7 +4569,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.lookup_inbound_stream(header.stream_identifier, header.type_); self.dispatch_frame( header, &bytes[FrameHeader::BYTE_SIZE..], diff --git a/src/runtime/api/bun/h2_frame_parser.zig b/src/runtime/api/bun/h2_frame_parser.zig index 33896f733e6..00819e9b45d 100644 --- a/src/runtime/api/bun/h2_frame_parser.zig +++ b/src/runtime/api/bun/h2_frame_parser.zig @@ -1998,10 +1998,8 @@ pub const H2FrameParser = struct { return data.len; }; - const settings = this.remoteSettings orelse this.localSettings; - - if (frame.length > settings.maxFrameSize) { - log("received data frame with length: {d} and max frame size: {d}", .{ frame.length, settings.maxFrameSize }); + if (frame.length > this.localSettings.maxFrameSize) { + log("received data frame with length: {d} and max frame size: {d}", .{ frame.length, this.localSettings.maxFrameSize }); this.sendGoAway(frame.streamIdentifier, ErrorCode.FRAME_SIZE_ERROR, "Invalid dataframe frame size", this.lastStreamID, true); return data.len; } @@ -2110,9 +2108,7 @@ pub const H2FrameParser = struct { this.sendGoAway(frame.streamIdentifier, ErrorCode.PROTOCOL_ERROR, "GoAway frame on stream", this.lastStreamID, true); return data.len; } - const settings = this.remoteSettings orelse this.localSettings; - - if (frame.length < 8 or frame.length > settings.maxFrameSize) { + if (frame.length < 8 or frame.length > this.localSettings.maxFrameSize) { this.sendGoAway(frame.streamIdentifier, ErrorCode.FRAME_SIZE_ERROR, "invalid GoAway frame size", this.lastStreamID, true); return data.len; } @@ -2374,8 +2370,7 @@ pub const H2FrameParser = struct { return data.len; }; - const settings = this.remoteSettings orelse this.localSettings; - if (frame.length > settings.maxFrameSize) { + if (frame.length > this.localSettings.maxFrameSize) { this.sendGoAway(frame.streamIdentifier, ErrorCode.FRAME_SIZE_ERROR, "invalid Headers frame size", this.lastStreamID, true); return data.len; } diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index 33e4540a708..0c8ebc196e9 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -1412,6 +1412,64 @@ pub enum DevHandlerId { MemoryVisualizer, } +/// DNS-rebinding guard for `/_bun/...` internal routes. A rebound origin +/// (`attacker.com` → 127.0.0.1) presents `Host: attacker.com`; rejecting +/// non-loopback / non-IP / non-configured hostnames prevents the attacker's +/// page from reading bundled source via same-origin fetch. +fn is_allowed_dev_host(dev: &DevServer, req: &Request) -> bool { + let Some(host) = req.header(b"host") else { + return false; + }; + let host = if host.first() == Some(&b'[') { + match strings::index_of_scalar(host, b']') { + Some(end) => &host[..=end], + None => host, + } + } else { + match strings::last_index_of_char(host, b':') { + Some(colon) => &host[..colon], + None => host, + } + }; + if strings::eql_case_insensitive_ascii(host, b"localhost", true) { + return true; + } + const DOT_LOCALHOST: &[u8] = b".localhost"; + if host.len() > DOT_LOCALHOST.len() + && strings::eql_case_insensitive_ascii( + &host[host.len() - DOT_LOCALHOST.len()..], + DOT_LOCALHOST, + true, + ) + { + return true; + } + let ip = if host.first() == Some(&b'[') && host.last() == Some(&b']') { + &host[1..host.len() - 1] + } else { + host + }; + if strings::is_ip_address(ip) { + return true; + } + if let Some(server) = dev.server.as_ref() { + if let crate::server::server_config::Address::Tcp { + hostname: Some(h), .. + } = &server.config().address + { + return strings::eql_case_insensitive_ascii(host, h.as_bytes(), true); + } + } + false +} + +fn host_forbidden(resp: AnyResponse) { + resp.corked(move || { + resp.write_status(b"403 Forbidden"); + resp.end(b"Blocked: Host header does not match the dev server", false); + }); +} + /// `extern "C"` trampoline: recovers `&mut DevServer` from user-data and wraps /// the raw `uws_res` as `AnyResponse`, then calls the handler for `ID`. extern "C" fn dev_route_tramp( @@ -1429,6 +1487,9 @@ extern "C" fn dev_route_tramp( } else { AnyResponse::TCP(res.cast::()) }; + if !matches!(ID, DevHandlerId::Request) && !is_allowed_dev_host(dev, req) { + return host_forbidden(resp); + } match ID { DevHandlerId::JsRequest => on_js_request(dev, req, resp), DevHandlerId::AssetRequest => on_asset_request(dev, req, resp), @@ -1533,6 +1594,9 @@ impl bun_uws_sys::web_socket::WebSocketUpgradeServer for D // SAFETY: uWS guarantees `res` is non-null and live for the upgrade // callback; `Response` is an opaque handle. let res = unsafe { &mut *res }; + if !is_allowed_dev_host(this, req) { + return host_forbidden(res.as_any_response()); + } let dw = bun_core::heap::into_raw(HmrSocket::new(this, res)); let _ = this.active_websocket_connections.insert(dw, ()); let _ = res.upgrade( @@ -5773,31 +5837,6 @@ impl DevServer { emit_edges!(&self.server_graph); Ok(()) } - - pub fn on_web_socket_upgrade( - &mut self, - res: &mut R, - req: &mut Request, - upgrade_ctx: &mut WebSocketUpgradeContext, - id: usize, - ) where - R: ResponseLike, // TODO(port): bun_uws::ResponseLike once upstream lands - { - debug_assert!(id == 0); - - let dw: Box = HmrSocket::new(self, res); - let dw_ptr: *mut HmrSocket = bun_core::heap::into_raw(dw); - self.active_websocket_connections - .put_no_clobber(dw_ptr, ()) - .expect("oom"); - res.upgrade::<*mut HmrSocket>( - dw_ptr, - req.header(b"sec-websocket-key").unwrap_or(b""), - req.header(b"sec-websocket-protocol").unwrap_or(b""), - req.header(b"sec-websocket-extension").unwrap_or(b""), - upgrade_ctx, - ); - } } // PORT NOTE: MessageId/IncomingMessageId/ConsoleLogKind/HmrTopic are defined diff --git a/src/runtime/node/net/BlockList.rs b/src/runtime/node/net/BlockList.rs index 3d039de911f..15ab09b017c 100644 --- a/src/runtime/node/net/BlockList.rs +++ b/src/runtime/node/net/BlockList.rs @@ -33,7 +33,14 @@ use core::sync::atomic::{AtomicU32, Ordering as AtomicOrdering}; use bun_core::{String as BunString, ZStr}; use bun_jsc::{CallFrame, JSGlobalObject, JSValue, JsCell, JsResult, StringJsc as _}; -use bun_threading::Mutex; +use bun_threading::{Guarded, Mutex}; + +/// Addresses of `BlockList` instances currently embedded in a live +/// `SerializedScriptValue` (one entry per serialize; removed by +/// `BlockList__onStructuredCloneDestroy`). Deserialize only honours pointers +/// present here so wire bytes from another process (IPC `advanced` mode, +/// `node:v8.deserialize`) cannot smuggle an arbitrary address through tag 251. +static SERIALIZED_REFS: Guarded> = Guarded::new(Vec::new()); use crate::node::util::validators; use crate::socket::socket_address::{SocketAddress, sockaddr}; @@ -73,6 +80,13 @@ pub struct BlockList { /// We cannot lock/unlock a mutex estimated_size: AtomicU32, + + /// Per-instance random identity, written into the structured-clone wire + /// alongside the address. Deserialize re-reads it from the live instance + /// (after [`SERIALIZED_REFS`] confirms the address is safe to dereference) + /// so wire bytes captured before this instance existed cannot match even if + /// the allocator reused the same address. + serialize_nonce: u64, } impl BlockList { @@ -101,6 +115,11 @@ impl BlockList { da_rules: JsCell::new(Vec::new()), mutex: Mutex::default(), estimated_size: AtomicU32::new(0), + serialize_nonce: { + let mut n = [0u8; 8]; + bun_core::csprng(&mut n); + u64::from_ne_bytes(n) + }, })); Ok(ptr) } @@ -380,6 +399,8 @@ impl BlockList { use bun_io::Write as _; let _guard = this.mutex.lock_guard(); this.ref_(); + let addr = std::ptr::from_ref::(this) as usize; + SERIALIZED_REFS.lock().push(addr); let mut writer = StructuredCloneWriter { ctx, impl_: write_bytes, @@ -388,7 +409,8 @@ impl BlockList { // Only the address is serialized; deserialize re-derives `*mut Self` // via int→ptr cast and never forms `&mut Self` (only `ref_()` + // `to_js_ptr`, both `&self`/raw-ptr), so `from_ref` provenance is fine. - _ = writer.write_int_le(std::ptr::from_ref::(this) as usize); + _ = writer.write_int_le(addr); + _ = writer.write_int_le(this.serialize_nonce); } // C++ codegen calls this with a live `*mut *mut u8` cursor and end pointer; the @@ -411,9 +433,9 @@ impl BlockList { let mut r = bun_io::FixedBufferStream::new(unsafe { bun_core::ffi::slice(*ptr, total_length) }); - let int = match r.read_int_le::() { - Ok(v) => v, - Err(_) => { + let (int, nonce) = match (r.read_int_le::(), r.read_int_le::()) { + (Ok(a), Ok(n)) => (a, n), + _ => { return Err(global.throw(format_args!( "BlockList.onStructuredCloneDeserialize failed" ))); @@ -424,7 +446,23 @@ impl BlockList { // SAFETY: `r.pos <= total_length` (`read_exact` bounds-checks via `checked_add`). *ptr = unsafe { (*ptr).add(r.pos) }; + if !SERIALIZED_REFS.lock().contains(&int) { + return Err(global.throw(format_args!( + "BlockList.onStructuredCloneDeserialize failed" + ))); + } + let this: *mut Self = int as *mut Self; + // SAFETY: presence in `SERIALIZED_REFS` (paired `ref_()`/`deref()`) + // guarantees `this` is a live `BlockList` allocation, so the field read + // is in-bounds. The nonce check then rejects wire bytes that name this + // address but were produced by a *different* instance that has since + // been freed and whose slot the allocator reused. + if unsafe { (*this).serialize_nonce } != nonce { + return Err(global.throw(format_args!( + "BlockList.onStructuredCloneDeserialize failed" + ))); + } // A single SerializedScriptValue can be deserialized multiple times // (e.g. BroadcastChannel fan-out), so each wrapper must own its own ref // instead of adopting the one taken in serialize. The serialize ref is @@ -447,6 +485,13 @@ bun_jsc::jsc_host_abi! { /// [`BlockList::on_structured_clone_serialize`]. #[unsafe(no_mangle)] pub unsafe fn BlockList__onStructuredCloneDestroy(ptr: *mut c_void) -> () { + let addr = ptr as usize; + { + let mut refs = SERIALIZED_REFS.lock(); + if let Some(i) = refs.iter().position(|&a| a == addr) { + refs.swap_remove(i); + } + } // SAFETY: `ptr` is the same `*mut BlockList` passed to // `on_structured_clone_serialize`; it stayed alive because that path // bumped the refcount. Dropping that ref here may free the box. diff --git a/src/runtime/node/path.rs b/src/runtime/node/path.rs index 938d3c74d23..24edae2192a 100644 --- a/src/runtime/node/path.rs +++ b/src/runtime/node/path.rs @@ -3240,6 +3240,12 @@ pub fn resolve_windows_t<'a, T: PathCharCwd>( buf_offset = buf_size; let first_part_len = first_part_end - first_part_start; buf_size += first_part_len; + if buf_size > tmp_buf.len() { + return Err(bun_sys::Error::from_code( + bun_sys::E::ENAMETOOLONG, + bun_sys::Tag::TODO, + )); + } // SAFETY: src/dst within live buffers; ptr::copy handles overlap. unsafe { core::ptr::copy( @@ -3254,6 +3260,12 @@ pub fn resolve_windows_t<'a, T: PathCharCwd>( let slice_len = j - last; buf_offset = buf_size; buf_size += slice_len; + if buf_size > tmp_buf.len() { + return Err(bun_sys::Error::from_code( + bun_sys::E::ENAMETOOLONG, + bun_sys::Tag::TODO, + )); + } // SAFETY: src/dst within live buffers; ptr::copy handles overlap. unsafe { core::ptr::copy( diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index 5da055bfd09..5c4f6cf98bf 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -3379,6 +3379,10 @@ where // we may not know the content-type when streaming && (!self.blob.is_detached() || content_type.value.as_ptr() != bun_http_types::MimeType::OTHER.value.as_ptr()) + && !content_type + .value + .iter() + .any(|&b| matches!(b, b'\r' | b'\n' | 0)) { resp.write_header(b"content-type", &content_type.value); } diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index a57e2f84d56..749440a6640 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -1993,7 +1993,7 @@ where } } - if sec_websocket_key_str.len == 0 { + if sec_websocket_key_str.len != 24 { return Ok(JSValue::FALSE); } if sec_websocket_protocol.len > 0 { diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 24c8b912667..607ce8df5f4 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -2709,9 +2709,15 @@ impl BlobExt for Blob { if bom == Some(strings::BOM::Utf16Le) { let _free = (LIFETIME == Lifetime::Temporary).then(|| TemporaryBytes(raw_bytes)); - // BOM::Utf16Le ⇒ buf is UTF-16LE bytes; len is even after BOM strip. - // Mirrors Zig `bun.reinterpretSlice(u16, buf)`; bytemuck checks align + even-len. + // Mirrors Zig `bun.reinterpretSlice(u16, buf)`: drop a trailing odd byte + // (`@divTrunc`) so `bytemuck::cast_slice` cannot panic on slop. // +1 WTF ref; `OwnedString` releases it on scope exit (Zig: `defer out.deref()`). + // + // ZIG PARITY: this branch intentionally does NOT `self.detach()` for + // `Lifetime::Transfer` — `Blob.zig` `toStringWithBytes` (UTF-16LE arm) + // returns without detaching, unlike `toJSONWithBytes`. Any change to + // that asymmetry belongs upstream in the Zig source, not here. + let buf = &buf[..buf.len() & !1]; let out = OwnedString::new(BunString::clone_utf16(bytemuck::cast_slice::(buf))); return out.to_js(global); @@ -2928,9 +2934,10 @@ impl BlobExt for Blob { } if bom == Some(strings::BOM::Utf16Le) { - // BOM::Utf16Le ⇒ buf is UTF-16LE bytes; len is even after BOM strip. - // Mirrors Zig `bun.reinterpretSlice(u16, buf)`; bytemuck checks align + even-len. + // Mirrors Zig `bun.reinterpretSlice(u16, buf)`: drop a trailing odd byte + // (`@divTrunc`) so `bytemuck::cast_slice` cannot panic on slop. // +1 WTF ref; `OwnedString` releases it on scope exit (Zig: `defer out.deref()`). + let buf = &buf[..buf.len() & !1]; let mut out = OwnedString::new(BunString::clone_utf16(bytemuck::cast_slice::(buf))); // PORT NOTE: Zig used `defer { free; detach }`. Reshaped to compute the @@ -3875,6 +3882,25 @@ struct FormDataContext<'a> { global_this: &'a JSGlobalObject, } +/// WHATWG HTML "multipart/form-data encoding algorithm": percent-encode `"`, +/// CR and LF in field names and filenames so they cannot terminate the +/// quoted-string or inject part headers. +fn escape_form_data_name(bytes: Vec) -> Box<[u8]> { + if !bytes.iter().any(|&b| matches!(b, b'"' | b'\r' | b'\n')) { + return bytes.into_boxed_slice(); + } + let mut out = Vec::with_capacity(bytes.len() + 6); + 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 { @@ -3895,7 +3921,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_name(name.to_slice().into_vec())); match entry { FormDataEntry::String(value) => { @@ -3904,11 +3930,14 @@ 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_name(filename.to_slice().into_vec())); joiner.push_static(b"\"\r\n"); - let content_type: &[u8] = if !blob.content_type_slice().is_empty() { - blob.content_type_slice() + let blob_ct = blob.content_type_slice(); + let content_type: &[u8] = if !blob_ct.is_empty() + && !blob_ct.iter().any(|&b| matches!(b, b'\r' | b'\n')) + { + blob_ct } else { b"application/octet-stream" }; diff --git a/src/runtime/webcore/s3/multipart.rs b/src/runtime/webcore/s3/multipart.rs index 674b5f5489a..075e2dd532a 100644 --- a/src/runtime/webcore/s3/multipart.rs +++ b/src/runtime/webcore/s3/multipart.rs @@ -692,8 +692,11 @@ impl MultiPartUpload { let slice = response.body.list.as_slice(); // PERF(port): Zig stored body and sliced upload_id into it; here we dupe upload_id if let Some(start) = strings::index_of(slice, b"") { + let value_start = start + b"".len(); if let Some(end) = strings::index_of(slice, b"") { - this.upload_id = Box::<[u8]>::from(&slice[start + 10..end]); + if end >= value_start { + this.upload_id = Box::<[u8]>::from(&slice[value_start..end]); + } } } this.uploadid_buffer = response.body; diff --git a/src/semver/lib.rs b/src/semver/lib.rs index 58ddce84f29..46db627f25b 100644 --- a/src/semver/lib.rs +++ b/src/semver/lib.rs @@ -508,15 +508,15 @@ 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) }, - ) + // Binary lockfiles raw-cast these offsets from disk; bounds-check in release. + // OOB handles must not normalize to "" (would make corrupt entries match). + match ( + this_buf.get(a_off..a_off + a_len), + that_buf.get(b_off..b_off + b_len), + ) { + (Some(a), Some(b)) => strings::eql(a, b), + _ => false, + } } } @@ -586,12 +586,8 @@ 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) } + // Binary lockfiles raw-cast these offsets from disk; bounds-check in release. + buf.get(off..off + len).unwrap_or_default() } } } diff --git a/src/sql/mysql/protocol/AnyMySQLError.rs b/src/sql/mysql/protocol/AnyMySQLError.rs index e18df83489e..e8a5031b346 100644 --- a/src/sql/mysql/protocol/AnyMySQLError.rs +++ b/src/sql/mysql/protocol/AnyMySQLError.rs @@ -12,6 +12,7 @@ pub enum Error { AuthenticationFailed, FailedToEncryptPassword, InvalidPublicKey, + PublicKeyRetrievalNotAllowed, UnsupportedAuthPlugin, UnsupportedProtocolVersion, diff --git a/src/sql_jsc/mysql/JSMySQLConnection.rs b/src/sql_jsc/mysql/JSMySQLConnection.rs index 3c94116216f..6fb2f49287f 100644 --- a/src/sql_jsc/mysql/JSMySQLConnection.rs +++ b/src/sql_jsc/mysql/JSMySQLConnection.rs @@ -581,6 +581,7 @@ impl JSMySQLConnection { let use_unnamed_prepared_statements = arguments[14].as_boolean(); // MySQL doesn't support unnamed prepared statements let _ = use_unnamed_prepared_statements; + let allow_public_key_retrieval = callframe.argument(15).to_boolean(); // Ownership transferred into `ptr.connection`; disarm the errdefer so the // connect-fail `ptr.deref()` is the sole cleanup path from here on. @@ -601,6 +602,7 @@ impl JSMySQLConnection { tls_config, secure, ssl_mode, + allow_public_key_retrieval, )), auto_flusher: JsCell::new(AutoFlusher::default()), idle_timeout_interval_ms: u32::try_from(idle_timeout).expect("int cast"), diff --git a/src/sql_jsc/mysql/MySQLConnection.rs b/src/sql_jsc/mysql/MySQLConnection.rs index 91c780a8375..53a44a3d49a 100644 --- a/src/sql_jsc/mysql/MySQLConnection.rs +++ b/src/sql_jsc/mysql/MySQLConnection.rs @@ -87,6 +87,7 @@ pub struct MySQLConnection { tls_config: SSLConfig, tls_status: TLSStatus, ssl_mode: SSLMode, + allow_public_key_retrieval: bool, flags: ConnectionFlags, } @@ -118,6 +119,7 @@ impl Default for MySQLConnection { tls_config: SSLConfig::default(), tls_status: TLSStatus::None, ssl_mode: SSLMode::Disable, + allow_public_key_retrieval: false, flags: ConnectionFlags::default(), } } @@ -138,6 +140,7 @@ impl MySQLConnection { tls_config: SSLConfig, secure: Option<*mut SslCtx>, ssl_mode: SSLMode, + allow_public_key_retrieval: bool, ) -> Self { Self { database, @@ -151,6 +154,7 @@ impl MySQLConnection { tls_config, secure, ssl_mode, + allow_public_key_retrieval, tls_status: if ssl_mode != SSLMode::Disable { TLSStatus::Pending } else { @@ -819,6 +823,15 @@ impl MySQLConnection { bun_core::scoped_log!(MySQLConnection, "continue auth"); if self.tls_status != TLSStatus::SslOk { + // Over plain TCP, an on-path attacker can answer the + // public-key request with their own key and recover the + // password. Match mysql2 / Connector/J: refuse unless the + // user explicitly opted in. + if !self.allow_public_key_retrieval { + return Err( + AnyMySQLError::PublicKeyRetrievalNotAllowed, + ); + } // we are in plain TCP so we need to request the public key self.set_status(ConnectionState::AuthenticationAwaitingPk); bun_core::scoped_log!( diff --git a/src/sql_jsc/mysql/protocol/any_mysql_error_jsc.rs b/src/sql_jsc/mysql/protocol/any_mysql_error_jsc.rs index 91b2ead6c91..dbda5a7cbce 100644 --- a/src/sql_jsc/mysql/protocol/any_mysql_error_jsc.rs +++ b/src/sql_jsc/mysql/protocol/any_mysql_error_jsc.rs @@ -113,6 +113,7 @@ pub fn mysql_error_to_js( "MissingAuthData" => b"ERR_MYSQL_MISSING_AUTH_DATA", "FailedToEncryptPassword" => b"ERR_MYSQL_FAILED_TO_ENCRYPT_PASSWORD", "InvalidPublicKey" => b"ERR_MYSQL_INVALID_PUBLIC_KEY", + "PublicKeyRetrievalNotAllowed" => b"ERR_MYSQL_PUBLIC_KEY_RETRIEVAL_NOT_ALLOWED", "InvalidState" => b"ERR_MYSQL_INVALID_STATE", "JSError" => { return global_object.take_exception(JsError::Thrown); diff --git a/src/sql_jsc/postgres/PostgresRequest.rs b/src/sql_jsc/postgres/PostgresRequest.rs index 4d668826d43..02c4054ed81 100644 --- a/src/sql_jsc/postgres/PostgresRequest.rs +++ b/src/sql_jsc/postgres/PostgresRequest.rs @@ -469,7 +469,10 @@ 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 { + 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/PostgresSQLConnection.rs b/src/sql_jsc/postgres/PostgresSQLConnection.rs index 43efdcaa4d2..129a983e73b 100644 --- a/src/sql_jsc/postgres/PostgresSQLConnection.rs +++ b/src/sql_jsc/postgres/PostgresSQLConnection.rs @@ -2710,6 +2710,13 @@ impl PostgresSQLConnection { } debug!("SASLContinue"); + // RFC 5802 §5.1: the server's combined nonce MUST begin with + // the client nonce we sent in the client-first-message. + if !cont.r.slice().starts_with(sasl.nonce()) { + debug!("SASLContinue server nonce does not start with client nonce"); + return Err(AnyPostgresError::InvalidMessage); + } + let iteration_count = cont.iteration_count().map_err(pg_err)?; // RFC 7677 §4: SCRAM-SHA-256 requires a minimum of 4096 // iterations. Cap the upper bound to avoid a CPU-burn DoS diff --git a/src/sys/tmp.rs b/src/sys/tmp.rs index 8314fdeb609..a2f70ffe131 100644 --- a/src/sys/tmp.rs +++ b/src/sys/tmp.rs @@ -57,7 +57,7 @@ impl<'a> Tmpfile<'a> { tmpfile.fd = crate::openat( destination_dir, tmpfilename, - O::CREAT | O::CLOEXEC | O::WRONLY, + O::CREAT | O::EXCL | O::CLOEXEC | O::WRONLY, perm, )? .make_lib_uv_owned_for_syscall(Tag::open, ErrorCase::CloseOnFail)?; diff --git a/src/valkey/valkey_protocol.rs b/src/valkey/valkey_protocol.rs index 5629ac6ff1e..fc218bcf7b2 100644 --- a/src/valkey/valkey_protocol.rs +++ b/src/valkey/valkey_protocol.rs @@ -291,7 +291,7 @@ impl<'a> ValkeyReader<'a> { pub fn read_verbatim_string(&mut self) -> Result { let len = self.read_integer()?; - if len < 0 { + if !(0..=Self::MAX_BULK_LEN).contains(&len) { return Err(RedisError::InvalidVerbatimString); } let len = usize::try_from(len).expect("int cast"); @@ -324,6 +324,13 @@ impl<'a> ValkeyReader<'a> { /// deeply nested responses. const MAX_NESTING_DEPTH: usize = 128; + /// Maximum accepted length for a single RESP blob (`$`, `=`, `!`). + /// Matches the Redis/Valkey server default `proto-max-bulk-len` of 512 MB. + /// Declared lengths above this fail the parse so the connection state + /// machine stops buffering instead of growing the read buffer toward an + /// attacker-chosen size. + const MAX_BULK_LEN: i64 = 512 * 1024 * 1024; + pub fn read_value(&mut self) -> Result { self.read_value_with_depth(0) } @@ -352,6 +359,9 @@ impl<'a> ValkeyReader<'a> { if len < 0 { return Ok(RESPValue::BulkString(None)); } + if len > Self::MAX_BULK_LEN { + return Err(RedisError::InvalidBulkString); + } let len = usize::try_from(len).expect("int cast"); if self.pos + len > self.buffer.len() { return Err(RedisError::InvalidResponse); @@ -399,7 +409,7 @@ impl<'a> ValkeyReader<'a> { } RESPType::BlobError => { let len = self.read_integer()?; - if len < 0 { + if !(0..=Self::MAX_BULK_LEN).contains(&len) { return Err(RedisError::InvalidBlobError); } let len = usize::try_from(len).expect("int cast"); diff --git a/test/cli/inspect/inspect.test.ts b/test/cli/inspect/inspect.test.ts index e810160ab0c..41315f70310 100644 --- a/test/cli/inspect/inspect.test.ts +++ b/test/cli/inspect/inspect.test.ts @@ -9,7 +9,7 @@ import { InspectorSession, JUnitReporter, connect } from "./junit-reporter"; import { SocketFramer } from "./socket-framer"; let inspectee: Subprocess; const anyPort = expect.stringMatching(/^\d+$/); -const anyPathname = expect.stringMatching(/^\/[a-z0-9]+$/); +const anyPathname = expect.stringMatching(/^\/[a-z0-9-]+$/); /** * Get a function that creates a random `.sock` file in the specified temporary directory. diff --git a/test/js/sql/sql-mysql.auth.test.ts b/test/js/sql/sql-mysql.auth.test.ts index f8d11ea8709..0eed6f6e80f 100644 --- a/test/js/sql/sql-mysql.auth.test.ts +++ b/test/js/sql/sql-mysql.auth.test.ts @@ -37,7 +37,24 @@ describeWithContainer( GRANT ALL PRIVILEGES ON bun_sql_test.* TO caching@'%'; FLUSH PRIVILEGES;`.simple(); } - await using sql = new SQL(`mysql://caching:bunbun@${container.host}:${container.port}/bun_sql_test`); + { + // Negative case: default (allowPublicKeyRetrieval unset) must refuse to fetch the server key. + // Must run before the successful login below so caching_sha2_password hasn't cached credentials yet. + await using denied = new SQL({ + url: `mysql://caching:bunbun@${container.host}:${container.port}/bun_sql_test`, + max: 1, + }); + const err = await denied`select 1 as x`.then( + () => null, + e => e, + ); + expect(err).not.toBeNull(); + expect(err?.code).toBe("ERR_MYSQL_PUBLIC_KEY_RETRIEVAL_NOT_ALLOWED"); + } + await using sql = new SQL({ + url: `mysql://caching:bunbun@${container.host}:${container.port}/bun_sql_test`, + allowPublicKeyRetrieval: true, + }); const result = await sql`select 1 as x`; expect(result).toEqual([{ x: 1 }]); await sql.end(); diff --git a/test/js/sql/sql-mysql.test.ts b/test/js/sql/sql-mysql.test.ts index 049709189d9..290856725b5 100644 --- a/test/js/sql/sql-mysql.test.ts +++ b/test/js/sql/sql-mysql.test.ts @@ -44,6 +44,7 @@ if (isDockerEnabled()) { const getOptions = (): Bun.SQL.Options => ({ url: `mysql://root:${password}@${container.host}:${container.port}/bun_sql_test`, max: 1, + allowPublicKeyRetrieval: true, tls: image.name === "MySQL with TLS" ? Bun.file(path.join(import.meta.dir, "mysql-tls", "ssl", "ca.pem")) diff --git a/test/regression/issue/malformed-integrity-base64.test.ts b/test/regression/issue/malformed-integrity-base64.test.ts index bd3db681c26..9b93b49820a 100644 --- a/test/regression/issue/malformed-integrity-base64.test.ts +++ b/test/regression/issue/malformed-integrity-base64.test.ts @@ -58,6 +58,8 @@ test("malformed integrity base64 in lockfile should be handled gracefully", asyn lodash@4.17.21 done" `); - expect(normalizeBunSnapshot(stderr.toString(), dir)).toMatchInlineSnapshot(`""`); + const err = normalizeBunSnapshot(stderr.toString(), dir); + expect(err).toContain("warn: Unsupported or malformed integrity hash; ignoring"); + expect(err).not.toContain("error:"); expect(exitCode).toMatchInlineSnapshot(`0`); });