Hardening: input validation and bounds checking across 12 subsystems (round 8)

packages/bun-debug-adapter-protocol/src/debugger/adapter.ts

@@ -224,6 +224,26 @@ export type DebugAdapterEventMap = InspectorEventMap & {
 const isDebug = process.env.NODE_ENV === "development";
 const debugSilentEvents = new Set(["Adapter.event", "Inspector.event"]);
 
+const inspectorEventDomains = new Set([
+  "Audit",
+  "Console",
+  "Debugger",
+  "Heap",
+  "Inspector",
+  "LifecycleReporter",
+  "Runtime",
+  "ScriptProfiler",
+  "TestReporter",
+]);
+
+function isInspectorEvent(event: unknown): boolean {
+  if (typeof event !== "string") {
+    return false;
+  }
+  const dot = event.indexOf(".");
+  return dot !== -1 && inspectorEventDomains.has(event.slice(0, dot));
+}
+
 let threadId = 1;
 
 // Add these helper functions at the top level
@@ -278,7 +298,9 @@ export abstract class BaseDebugAdapter<T extends Inspector = Inspector>
     this.inspector.emit = (event, ...args) => {
       let sent = false;
       sent ||= emit(event, ...args);
-      sent ||= this.emit(event as keyof JSC.EventMap, ...(args as any));
+      if (isInspectorEvent(event)) {
+        sent ||= this.emit(event as keyof JSC.EventMap, ...(args as any));
+      }
       return sent;
     };
     this.#sourceId = 1;

packages/bun-debug-adapter-protocol/src/debugger/signal.ts

@@ -132,7 +132,7 @@ export class TCPSocketSignal extends EventEmitter {
     });
 
     this.#ready = new Promise((resolve, reject) => {
-      this.#server.listen(this.#port, () => {
+      this.#server.listen(this.#port, "127.0.0.1", () => {
         this.emit("Signal.listening");
         resolve();
       });

packages/bun-debug-adapter-protocol/src/debugger/sourcemap.test.ts

@@ -1,5 +1,9 @@
 import { expect, test } from "bun:test";
 import { readFileSync } from "node:fs";
+import { connect } from "node:net";
+import { networkInterfaces } from "node:os";
+import { getAvailablePort, WebSocketDebugAdapter } from "./adapter.js";
+import { TCPSocketSignal } from "./signal.js";
 import { SourceMap } from "./sourcemap.js";
 
 test("works without source map", () => {
@@ -29,3 +33,82 @@ function getSourceMap(filename: string): SourceMap {
   }
   return SourceMap();
 }
+
+test("only forwards inspector events from known protocol domains to the adapter", () => {
+  const adapter = new WebSocketDebugAdapter();
+
+  // Replace the launch request handler so a (wrongly) forwarded event is observable
+  // without spawning any process.
+  const launchCalls: unknown[][] = [];
+  (adapter as any).launch = (...args: unknown[]) => {
+    launchCalls.push(args);
+  };
+
+  const inspector = adapter.getInspector();
+
+  // The WebSocket inspector re-emits any message without an "id" using the method name
+  // chosen by the remote peer. An event named after a DAP request must not be forwarded
+  // to the adapter, where it would be dispatched to the matching request handler.
+  (inspector as any).emit("launch", {
+    runtime: "/bin/sh",
+    runtimeArgs: ["-c", "echo unexpected"],
+    program: "example.js",
+  });
+  expect(launchCalls).toHaveLength(0);
+
+  // A genuine inspector-domain event still reaches listeners registered on the adapter.
+  const heapEvents: unknown[] = [];
+  adapter.on("Heap.garbageCollected", event => {
+    heapEvents.push(event);
+  });
+  (inspector as any).emit("Heap.garbageCollected", {
+    collection: { type: "full", startTime: 0, endTime: 1 },
+  });
+  expect(heapEvents).toEqual([{ collection: { type: "full", startTime: 0, endTime: 1 } }]);
+});
+
+test("TCPSocketSignal accepts connections only on the loopback interface", async () => {
+  // Same construction the VS Code extension uses (diagnostics.ts createSignal).
+  const port = await getAvailablePort();
+  const signal = new TCPSocketSignal(port);
+  await signal.ready;
+
+  try {
+    // The legitimate local client connects over loopback and its payload is delivered.
+    const received = new Promise<string>(resolve => signal.once("Signal.received", resolve));
+    await new Promise<void>((resolve, reject) => {
+      const client = connect({ host: "127.0.0.1", port }, () => {
+        client.end("hello");
+        resolve();
+      });
+      client.on("error", reject);
+    });
+    expect(await received).toBe("hello");
+
+    // The same port is not reachable through a non-loopback interface address.
+    let external: string | undefined;
+    for (const addresses of Object.values(networkInterfaces())) {
+      for (const { family, internal, address } of addresses ?? []) {
+        if (family === "IPv4" && !internal) {
+          external = address;
+          break;
+        }
+      }
+      if (external) break;
+    }
+
+    if (external) {
+      const externalHost = external;
+      const connectError = await new Promise<Error | null>(resolve => {
+        const client = connect({ host: externalHost, port }, () => {
+          client.end();
+          resolve(null);
+        });
+        client.on("error", error => resolve(error));
+      });
+      expect(connectError).not.toBeNull();
+    }
+  } finally {
+    signal.close();
+  }
+});

src/exe_format/macho.rs

@@ -131,7 +131,10 @@ impl MachoFile {
                                     found_bun = true;
                                     original_fileoff = sect.offset as u64;
                                     let original_vmaddr = sect.addr;
-                                    original_data_end = command.fileoff + command.filesize;
+                                    original_data_end = command
+                                        .fileoff
+                                        .checked_add(command.filesize)
+                                        .ok_or(MachoError::OffsetOverflow)?;
                                     original_segsize = command.filesize;
                                     self.segment = command;
                                     self.section = sect;
@@ -207,6 +210,15 @@ impl MachoFile {
         let mut sig_size: usize = 0;
 
         let prev_len = self.data.len();
+        let original_bun_end = usize::try_from(original_fileoff)
+            .ok()
+            .and_then(|off| off.checked_add(usize::try_from(original_segsize).ok()?))
+            .ok_or(MachoError::OffsetOverflow)?;
+        let original_data_end =
+            usize::try_from(original_data_end).map_err(|_| MachoError::OffsetOverflow)?;
+        if original_bun_end > prev_len || original_data_end > original_bun_end {
+            return Err(MachoError::OffsetOutOfRange);
+        }
         // SAFETY: we just reserved `size_diff` bytes; new_len <= capacity. The newly-exposed bytes
         // are written below before being read (memmove + memset cover the whole range).
         unsafe {
@@ -217,19 +229,17 @@ impl MachoFile {
         // Binary is:
         // [header][...data before __BUN][__BUN][...data after __BUN]
         // We need to shift [...data after __BUN] forward by size_diff bytes.
-        // SAFETY: source and destination overlap; ptr::copy (memmove) handles this. Ranges are
-        // within self.data per the offset arithmetic above.
+        // SAFETY: source and destination overlap; ptr::copy (memmove) handles this.
+        // `original_bun_end <= prev_len` and `original_data_end <= original_bun_end` were
+        // checked above, so the source range stays within the previously-initialized bytes
+        // and the destination range stays within the new length `prev_len + size_diff`.
         unsafe {
             let after_bun_dst = self
                 .data
                 .as_mut_ptr()
-                .add((original_data_end as usize) + usize::try_from(size_diff).expect("int cast"));
-            let prev_after_bun_src = self
-                .data
-                .as_ptr()
-                .add(original_fileoff as usize + original_segsize as usize);
-            let prev_after_bun_len =
-                prev_len - (original_fileoff as usize + original_segsize as usize);
+                .add(original_data_end + usize::try_from(size_diff).expect("int cast"));
+            let prev_after_bun_src = self.data.as_ptr().add(original_bun_end);
+            let prev_after_bun_len = prev_len - original_bun_end;
             core::ptr::copy(prev_after_bun_src, after_bun_dst, prev_after_bun_len);
         }
 

src/install/extract_tarball.rs

@@ -849,7 +849,9 @@ impl ExtractTarball {
                 .unwrap_or(false)
             {
                 // create an index storing each version of a package installed
-                if strings::index_of_char(basename, b'/').is_none() {
+                if strings::index_of_char(basename, b'/').is_none()
+                    && bun_install::dependency::is_safe_install_folder_name(name)
+                {
                     'create_index: {
                         let dest_name: &[u8] = match self.resolution.tag {
                             ResolutionTag::Github => &folder_name[b"@GH@".len()..],

src/install/lockfile.rs

@@ -653,6 +653,29 @@ impl Lockfile {
             }
         };
 
+        for res in self.packages.items_resolution() {
+            if res.tag != ResolutionTag::Git && res.tag != ResolutionTag::Github {
+                continue;
+            }
+            let resolved = self.str(&res.repository().resolved);
+            if !resolved.is_empty() && !crate::repository::is_safe_resolved_tag(resolved) {
+                log.add_error_fmt(
+                    None,
+                    bun_ast::Loc::EMPTY,
+                    format_args!(
+                        "Invalid git dependency tag \"{}\" in bun.lockb",
+                        bstr::BStr::new(resolved)
+                    ),
+                );
+                return LoadResult::Err(LoadResultErr {
+                    step: LoadStep::ParseFile,
+                    value: err!("InvalidLockfile"),
+                    lockfile_path: zstr!("bun.lockb"),
+                    format: LockfileFormat::Binary,
+                });
+            }
+        }
+
         if cfg!(debug_assertions) {
             self.verify_data().expect("lockfile data is corrupt");
         }

src/js/node/_http2_upgrade.ts

@@ -344,6 +344,8 @@ function upgradeRawSocketToH2(
         cert: server.cert,
         ca: server.ca,
         passphrase: server.passphrase,
+        requestCert: server._requestCert,
+        rejectUnauthorized: server._rejectUnauthorized,
         ALPNProtocols: server.ALPNProtocols
           ? server.ALPNProtocols.buffer.slice(
               server.ALPNProtocols.byteOffset,

src/js/node/_http_client.ts

@@ -61,6 +61,7 @@ const { OutgoingMessage } = require("node:_http_outgoing");
 const globalReportError = globalThis.reportError;
 const setTimeout = globalThis.setTimeout;
 const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
+const INVALID_HOST_CHAR_REGEX = /[/\\?#@\t\n\r]/;
 
 const { URL } = globalThis;
 
@@ -527,6 +528,14 @@ function ClientRequest(input, options, cb) {
 
     if (isIP(host) || !options.lookup) {
       // Don't need to bother with lookup if it's already an IP address or no lookup function is provided.
+      if (RegExpPrototypeExec.$call(INVALID_HOST_CHAR_REGEX, host) !== null) {
+        const error = new Error(`getaddrinfo ENOTFOUND ${host}`);
+        error.name = "DNSException";
+        error.code = "ENOTFOUND";
+        error.syscall = "getaddrinfo";
+        process.nextTick((self, err) => self.emit("error", err), this, error);
+        return false;
+      }
       const [url, proxy] = getURL(host);
       go(url, proxy, false);
       return true;

src/jsc/bindings/v8/V8String.cpp

@@ -95,11 +95,11 @@ int String::Utf8Length(Isolate* isolate) const
     if (str->is8Bit()) {
         const auto span = str->span8();
         size_t len = simdutf::utf8_length_from_latin1(reinterpret_cast<const char*>(span.data()), span.size());
-        return static_cast<int>(len);
+        return static_cast<int>(std::min(len, static_cast<size_t>(std::numeric_limits<int>::max())));
     } else {
         const auto span = str->span16();
         size_t len = simdutf::utf8_length_from_utf16(span.data(), span.size());
-        return static_cast<int>(len);
+        return static_cast<int>(std::min(len, static_cast<size_t>(std::numeric_limits<int>::max())));
     }
 }
 
@@ -162,7 +162,7 @@ int String::WriteUtf8(Isolate* isolate, char* buffer, int length, int* nchars_re
     auto jsString = localToObjectPointer<JSString>();
     WTF::String string = jsString->getString(isolate->globalObject());
 
-    size_t unsigned_length = length < 0 ? SIZE_MAX : length;
+    size_t unsigned_length = length < 0 ? static_cast<size_t>(std::numeric_limits<int>::max()) : static_cast<size_t>(length);
 
     uint64_t result = string.is8Bit() ? TextEncoder__encodeInto8(string.span8().data(), string.span8().size(), buffer, unsigned_length)
                                       : TextEncoder__encodeInto16(string.span16().data(), string.span16().size(), buffer, unsigned_length);
@@ -173,7 +173,7 @@ int String::WriteUtf8(Isolate* isolate, char* buffer, int length, int* nchars_re
         buffer[written] = 0;
         written++;
     }
-    if (read < string.length() && U16_IS_SURROGATE(string[read]) && written + 3 <= length) {
+    if (read < string.length() && U16_IS_SURROGATE(string[read]) && written + 3 <= unsigned_length) {
         // encode unpaired surrogate
         char16_t surrogate = string[read];
         buffer[written + 0] = 0xe0 | (surrogate >> 12);

src/jsc/node_path.rs

@@ -141,9 +141,13 @@ impl Clone for PathLike {
 impl Drop for PathLike {
     fn drop(&mut self) {
         match self {
-            // `PathString` is a borrowed (ptr,len) pair; `MarkedArrayBuffer`
-            // is JS-GC-owned. Neither needs an explicit release here.
-            Self::String(_) | Self::Buffer(_) => {}
+            Self::String(_) => {}
+            Self::Buffer(b) => {
+                if b.pinned {
+                    b.pinned = false;
+                    b.buffer.unpin();
+                }
+            }
             Self::SliceWithUnderlyingString(s) | Self::ThreadsafeString(s) => {
                 core::mem::take(s).deinit();
             }

src/runtime/node/node_fs.rs

@@ -3995,11 +3995,19 @@ pub mod args {
             };
 
             // length |= 0;
-            let length_float: f64 = if let Some(arg) = arguments.next_eat() {
+            let length_value = arguments.next_eat();
+            let length_float: f64 = if let Some(arg) = length_value {
                 arg.to_number(ctx)?
             } else {
                 0.0
             };
+            let buffer = if length_value.is_some_and(|arg| !arg.is_number()) {
+                Buffer::from_js(ctx, buffer_value).ok_or_else(|| {
+                    ctx.throw_invalid_argument_type_value(b"buffer", b"TypedArray", buffer_value)
+                })?
+            } else {
+                buffer
+            };
 
             //   if (length === 0) {
             //     return process.nextTick(function tick() {

src/runtime/node/path.rs

@@ -1211,7 +1211,7 @@ pub(crate) fn format_js_t<T: PathCharCwd>(
         }) + (if base_len > 0 {
             base_len
         } else {
-            path_object.name.len() + path_object.ext.len()
+            path_object.name.len() + path_object.ext.len() + 1
         }))
         .max(path_size::<T>());
     let mut scratch = PathScratch::<T>::new(pool, buf_len);

src/runtime/node/types.rs

@@ -1220,6 +1220,12 @@ impl PathLikeExt for PathLike {
                 Valid::path_buffer(&buffer, ctx)?;
                 Valid::path_null_bytes(buffer.slice(), ctx)?;
 
+                let buffer = if arguments.will_be_async {
+                    Buffer::from_js_pinned(ctx, arg).unwrap_or(buffer)
+                } else {
+                    buffer
+                };
+
                 arguments.protect_eat();
                 Ok(Some(Self::Buffer(buffer)))
             }
@@ -1229,6 +1235,12 @@ impl PathLikeExt for PathLike {
                 Valid::path_buffer(&buffer, ctx)?;
                 Valid::path_null_bytes(buffer.slice(), ctx)?;
 
+                let buffer = if arguments.will_be_async {
+                    Buffer::from_js_pinned(ctx, arg).unwrap_or(buffer)
+                } else {
+                    buffer
+                };
+
                 arguments.protect_eat();
                 Ok(Some(Self::Buffer(buffer)))
             }

src/runtime/webcore/s3/multipart.rs

@@ -702,7 +702,10 @@ impl MultiPartUpload {
                 this.uploadid_buffer = response.body;
                 if this.upload_id.is_empty()
                     || this.upload_id.len() > Self::MAX_UPLOAD_ID_LEN
-                    || this.upload_id.iter().any(|b| b.is_ascii_control())
+                    || this
+                        .upload_id
+                        .iter()
+                        .any(|b| !b.is_ascii() || b.is_ascii_control())
                 {
                     // Unknown type of response error from AWS
                     scoped_log!(