Bun.udpSocket, Bun.password: range-check numeric options before ToInt32 narrowing

[robobun]

What does this PR do?

Bun.udpSocket ports and Bun.password cost options were coerced through ECMAScript ToInt32 (modular wrap) and range-checked only after the wrap, so out-of-range values beyond the i32 range silently passed:

await Bun.udpSocket({ port: 4294967377 });              // bound port 81 (2^32 + 81)
await Bun.udpSocket({ connect: { hostname: "127.0.0.1", port: 4294967377 } }); // peer port 81
socket.send("hi", 2 ** 32 + 9, "127.0.0.1");            // delivered to port 9
await Bun.udpSocket({ port: 80.5 });                    // bound port 80
Bun.password.hashSync("pw", { algorithm: "bcrypt", cost: 4294967300 }); // "$2b$04$..." (cost 4)
Bun.password.hashSync("pw", { algorithm: "argon2id", timeCost: 2 ** 32 + 2 }); // t=2

Meanwhile { port: 65617 } correctly threw Expected "port" to be an integer between 0 and 65535, because 65617 stays in i32 range.

How did you fix it?

Validate the double as an in-range integer before narrowing, at each site:

• udp_socket.rs: bind port (0..=65535), connect.port (1..=65535), and parse_addr for send/sendMany (1..=65535). parse_addr now throws for out-of-range ports instead of rewriting them to 0: on Linux the port-0 sockaddr happened to fail with EINVAL, but on Windows send() reported success for a datagram that never went anywhere. Port 0 stays allowed inside parse_addr because the multicast membership/interface callers pass a literal 0.
• PasswordObject.rs: bcrypt cost (4..=31), argon2 timeCost/memoryCost (1/8 ..= u32 max). Existing error messages are kept for the previously-rejected cases (Memory cost must be at least 8, Time cost must be greater than 0); wrapping and fractional values now throw instead of hashing with a truncated parameter.
• dgram.ts: Socket.prototype.bind(cb) kept the callback in its port variable and port || 0 forwarded the function to Bun.udpSocket, relying on the old ToInt32 coercion to turn it into 0 (Node does the same dance via ToUint32 in its native layer). Normalize a function first argument to "no port" in the JS layer; caught by test-dgram-socket-buffer-size.js and the other bind(cb) tests in the Node suite.

node:dgram's send/connect paths validate ports in JS (ERR_SOCKET_BAD_PORT) before reaching this code, so they were unaffected.

Tests

• test/js/bun/udp/udp_socket.test.ts: wrap values added to the existing connect.port case list, a new bind case list (4294967377, -4294967215, 2^53, 80.5, NaN, Infinity), and a send/sendMany test that binds a real socket and proves 2 ** 32 + server.port throws instead of delivering to server.port.
• test/js/bun/util/password.test.ts: wrap and fractional values for cost, timeCost, memoryCost across both hash and hashSync.

All fail on the released binary (10 udp + 2 password failures) and pass with this change; the full udp, dgram, and password suites plus all 75 test-dgram-* Node parallel tests pass with the debug build.

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/udp/udp_socket.test.ts test/js/bun/util/password.test.ts

[coderabbitai]

Walkthrough

The changes normalize callback-only datagram binding and validate UDP ports and password-hashing options before integer conversion. Regression tests cover fractional, non-finite, wrapped, unsafe, and out-of-range values.

Changes

Dgram binding

Layer / File(s)	Summary
Callback-only bind normalization src/js/node/dgram.ts	Socket.prototype.bind treats a function as the callback-only overload and sets the port to null.

Numeric validation

Layer / File(s)	Summary
UDP port validation src/runtime/socket/udp_socket.rs, test/js/bun/udp/udp_socket.test.ts	UDP port parsing validates numeric range and integer status before conversion. Tests cover bind, connect, send, and sendMany cases.
Password option validation src/runtime/crypto/PasswordObject.rs, test/js/bun/util/password.test.ts	Bcrypt and Argon2 options reject fractional, non-finite, and out-of-range values before integer conversion. Tests cover each option range.

Possibly related PRs

• oven-sh/bun#36998: Both PRs validate numeric inputs before narrowing conversions to prevent overflow, wrapping, or truncation.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main numeric range-validation changes in Bun.udpSocket and Bun.password.
Description check	✅ Passed	The description includes both required sections and clearly explains the changes, fixes, affected APIs, and verification results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/socket/udp_socket.rs`:
- Around line 1576-1586: The port parsing logic around parse_addr currently
accepts port 0 for all callers; restrict that behavior to membership and
interface parsing only. Add an allow_zero parameter or dedicated helper, enable
it exclusively in those callers, and ensure send and send_many_impl reject zero
while preserving the 1–65535 validation. Extend the send and sendMany regression
tests with port-zero cases.

In `@test/js/bun/udp/udp_socket.test.ts`:
- Around line 109-125: The test currently creates client outside the cleanup
scope, leaving server open if client creation fails. Move client creation into
the existing try/finally flow around the test, or use nested cleanup so
server.close() always runs while client.close() only runs after successful
creation.

In `@test/js/bun/util/password.test.ts`:
- Around line 142-186: Add assertions in the existing “cost values are
range-checked before ToInt32 narrowing” test for Argon2 `timeCost` and
`memoryCost` with `NaN`, `-Infinity`, and `Infinity`. Each case should call
`hash` with the corresponding algorithm and option and assert the existing
field-specific validation error, covering both non-finite and range checks.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 44851981-4572-4885-b91a-ba0f68e4586d

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb02aa and bc0ed75.

📒 Files selected for processing (5)

• src/js/node/dgram.ts
• src/runtime/crypto/PasswordObject.rs
• src/runtime/socket/udp_socket.rs
• test/js/bun/udp/udp_socket.test.ts
• test/js/bun/util/password.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/socket/udp_socket.rs`:
- Around line 1542-1552: Update UDPSocket::js_connect to validate its port
argument through parse_port before narrowing or initiating any socket side
effects, replacing the existing as_int32 conversion. Preserve valid integer
ports while rejecting fractional, wrapped/out-of-range, zero, NaN, and infinite
values; add dgram.connect() coverage for these cases.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68a44651-5e1d-40e4-8c93-8b610cbcebc4

📥 Commits

Reviewing files that changed from the base of the PR and between bc0ed75 and 6258edc.

📒 Files selected for processing (3)

• src/runtime/socket/udp_socket.rs
• test/js/bun/udp/udp_socket.test.ts
• test/js/bun/util/password.test.ts

[robobun]

CI status on 6258edc: every test this PR touches is green on all lanes (udp, dgram, password suites). The remaining failures are unrelated: the x64-asan lane fails on an RSA keygen leak in test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts that also fails on main (reported separately), and the rest are known-flaky lanes that passed on retry (dns, install, dlopen). Ready for review.

[claude]

LGTM — focused input-validation fix that range-checks numeric options as f64 before narrowing.

What was reviewed:

• f64 range/fract checks in PasswordObject.rs and udp_socket.rs: NaN and ±Infinity are caught (.fract() on non-finite is NaN, and NaN != 0.0 is true; range .contains() on NaN is false); as u8/u16/u32 casts are sound after the checks.
• parse_addr refactor to take a validated u16 — membership/interface callers pass literal 0, send/sendMany go through parse_port (rejects 0), so port 0 no longer reaches sendto.
• dgram.ts bind(cb) normalization — needed because the stricter native check would otherwise see the callback as the port and throw; matches Node's overload.
• Ruled out: coerce_f64 string coercion still accepts "80" (ToNumber → 80.0), so numeric-string ports keep working; u32::MAX is exactly representable in f64 so the upper-bound compare is exact.

Extended reasoning...

Overview

The PR fixes silent ToInt32 wrapping in Bun.udpSocket port options (bind, connect, send/sendMany) and Bun.password cost options (bcrypt cost, argon2 timeCost/memoryCost). Values like 2**32 + 81 previously wrapped to 81 and passed the post-narrowing range check; now the f64 is validated first (fract() == 0 and in-range) before the as cast. parse_addr was refactored to take an already-validated u16 port, with a new parse_port helper for the send paths, so the port-0 special case for membership callers becomes a plain literal instead of a branch. A one-line fix in dgram.ts normalizes the bind(cb) overload so the callback isn't forwarded as the port (which the old ToInt32 path silently coerced to 0).

Security risks

None introduced. The change strictly tightens input validation on user-supplied numbers — wrapped/fractional/non-finite values now throw instead of silently binding/sending to an unintended port or hashing with a truncated cost parameter. This is a mild security improvement (a wrapped port could have sent data to port 81 when the caller wrote 2**32 + 81).

Level of scrutiny

Medium. Native code touching JSC value coercion and sockaddr construction, but the changes are localized to the validation step and don't touch memory layout, lifetime, or the send path itself. I verified each f64 check handles NaN and ±Infinity correctly (Rust's f64::fract() on non-finite yields NaN, and NaN != 0.0 is true; RangeInclusive::contains(&NaN) is false). The as u16/as u8/as u32 casts are sound because the preceding range check guarantees the value is an in-range integer, and u32::MAX is exactly representable in f64.

Other factors

All prior review feedback (comment-cop long-comment flags, CodeRabbit's port-0 restriction, test cleanup scoping, non-finite argon2 cases) has been addressed and the threads are resolved. The one remaining CodeRabbit note about js_connect's as_int32() was withdrawn — that path is guarded by validatePort in JS and is the subject of a separate open PR. Test coverage is thorough: wrap values on both sides of zero, 2**53, fractional, NaN, ±Infinity, and port 0 for send/sendMany. The send/sendMany behavior change (throw instead of silent clamp-to-0) is the intended fix and matches Node's validatePort(allowZero: false).