Skip to content

bindgen v2: do not read dictionary members from Object.prototype - #44824

Closed
robobun wants to merge 5 commits into
mainfrom
robobun/727189fc/bindgenv2-dictionary-proto-pollution
Closed

robobun wants to merge 5 commits into
mainfrom
robobun/727189fc/bindgenv2-dictionary-proto-pollution

Conversation

@robobun

@robobun robobun commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Bun.serve tls, Bun.listen, Bun.connect and fetch tls take options from Object.prototype. Object.prototype.keyFile = "/x" makes Bun.serve({ tls: { cert, key } }) throw Unable to access keyFile path. Object.prototype.timeout = 5000 makes each Bun.connect throw Expected "onTimeout" callback to be a function.
  • The cause is src/codegen/bindgenv2/internal/dictionary.ts:317. The generated converter reads each member with object->get(), which also looks in Object.prototype.
  • Regression from Add new bindings generator; port SSLConfig #23169 (1.3.0) and Port SocketConfig to bindings generator #23755 (1.3.1). Before them, Zig getTruthy read these options and stopped before Object.prototype. Bun 1.2.23 ignores both values.

Fix

  • The converter reads each member with Bun::getIfPropertyExistsPrototypePollutionMitigation, which stops before Object.prototype.
  • Correct because bindgen v1 (src/codegen/bindgen.ts:665) and Rust JSValue::get use the same getter.
  • Verified: four new cases in test/js/bun/net/socket.test.ts and test/js/bun/http/bun-serve-ssl.test.ts fail on 1.4.3-canary and pass here. Also ran test/js/node/tls and test/js/node/net.

Background

  • Prototype pollution: a bug in an app or a dependency sets a property on Object.prototype, so each plain object appears to have it.
  • bindgen v2 generates the C++ that converts an options object to a struct for Rust. It has four: TLSOptions, SocketOptions, SocketHandler, FakeTimersOptions.
  • Considered a flag per dictionary and a copy of the object in each caller. Both leave the generator default unsafe.

Downsides

  • Code that sets a TLS or socket option on Object.prototype on purpose loses that.
  • node:tls reads its options in JavaScript, like Node. Object.prototype.servername still sets the SNI of tls.connect(), as in Node v26.3.0.
  • Cost: release text grows by 512 bytes. A loop that converts three dictionaries per call is 53 ns slower per call, below the 800 ns noise floor.
Notes

Status: draft until the self-review of the diff is done.

A/B, same script, a data value on Object.prototype in each case

case bun 1.2.23 1.4.3-canary.1+367d939d9 this branch
Bun.serve({ tls: { cert, key } }) + keyFile = "/nonexistent/attacker.key" ok throws Unable to access keyFile path ok
Bun.connect({ socket: { data } }) + timeout = 5000 ok throws Expected "onTimeout" callback to be a function ok
Bun.connect with no tls member + tls = true peer receives PLAINTEXT peer receives a TLS ClientHello peer receives PLAINTEXT

The tests add three more: a data handler on Object.prototype receives the bytes of a client that has no data handler, rejectUnauthorized = false lets Bun.connect({ tls: { serverName } }) accept a self-signed certificate, and the same value lets a Bun.serve server with requestCert: true answer a client that has no certificate.

History. git show f14f3b03bb^:src/bun.js/api/server/SSLConfig.zig reads each member with obj.getTruthy(...), getBooleanStrict or get. git show 4a06991d3b^:src/bun.js/api/bun/socket/Handlers.zig uses getTruthyComptime. Those call JSC__JSValue__getIfPropertyExistsImpl, which calls Bun::getIfPropertyExistsPrototypePollutionMitigationUnsafe. git tag --contains gives bun-v1.3.0 for the first port and bun-v1.3.1 for the second.

What does not change (same output on 1.2.23, 1.4.3-canary and this branch, the last one with BUN_JSC_validateExceptionChecks=1):

  • Handlers that live on a class prototype still run. The lookup walks the prototype chain and stops only at Object.prototype.
  • A null-prototype object and a Proxy work as an options object. The get trap sees the same keys.
  • A getter that throws on a member of any of the three socket dictionaries rethrows to the caller.

Cost, measured on Linux x64.

  • Size: size build/release/bun on main (116dd64) with and without the emitter change. text 106,027,183 to 106,027,695 (+512). data 115,224 and bss 1,823,240 do not change.
  • Time: a loop of Bun.listen({ socket: {...}, tls: {} }) with no hostname. Each call converts SocketOptions, SocketHandler and TLSOptions (51 member reads) and then throws Expected either "hostname" or "unix". 11 interleaved runs of 300,000 calls for each release binary: median 2974 ns per call before and 3027 ns after (+53 ns, +1.8%). Runs of the same binary differ by up to 800 ns, so the difference is below the noise.
  • perf and valgrind are not installed in my environment, so there is no instruction count. The getter (src/jsc/bindings/ObjectBindings.cpp) fills a PropertySlot on the stack and allocates nothing.

Local test runs (debug + ASAN build, Linux x64). On the first version of this branch (based on 620b50f) I ran test/js/node/tls, test/js/node/net, socket.test.ts, tcp-server.test.ts and bun-serve-ssl.test.ts on a build of main and on the branch: 56 failures on main, 23 on the branch. All 23 are timeouts at the 5 s default or tests that need the public internet, and 22 of them also fail on main. The other one (kqueue should not dispatch spurious drain events on readable) passes 3 of 3 times when it runs alone. After the merge with main (116dd64) I ran the four new cases again: they pass on the branch and fail on 1.4.3-canary. Not run: macOS, Windows.

Bun.SQL is not in this PR. Its option normaliser (src/js/internal/sql/shared.ts) also reads from Object.prototype: Object.prototype.adapter = "sqlite" makes new Bun.SQL("postgres://...") use SQLite. That is not a regression (1.2.23 does the same), and the cause is in JavaScript, not in the generator. A null-prototype copy in the normaliser does not close it: createPooledConnectionHandle later reads path, idleTimeout, connectionTimeout and maxLifetime from the normalised object, which is an ordinary object that has those keys only when they are set. With Object.prototype.path set after new Bun.SQL("postgres://...") returns, the client sends its StartupMessage to that unix socket. A complete fix changes the shape of sql.options or separates it from the state of the pool, so it is a separate change.


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/net/socket.test.ts

The bindgen v2 dictionary converter read each member with a plain
[[Get]], so a polluted Object.prototype supplied any option the caller
did not pass. Read members with
getIfPropertyExistsPrototypePollutionMitigation, the same lookup the
v1 generator and JSValue::get use. This covers TLSOptions (Bun.serve,
Bun.listen, Bun.connect, fetch tls, Bun.SQL, RedisClient), SocketOptions
and SocketHandler.
node:https and node:net take about 2 s to load in a debug build, which
put the Bun.serve case at 4.3 s against the 5 s default timeout. The
cases now use only native APIs in the child and run in 0.8 to 1.5 s.
@robobun

robobun commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced with a script that sets one data value on Object.prototype and then calls one Bun API. Run on Linux x64.

case bun 1.2.23 1.4.3-canary.1+367d939d9 this branch
Bun.serve({ tls: { cert, key } }) with Object.prototype.keyFile = "/nonexistent/attacker.key" ok throws Unable to access keyFile path ok
Bun.connect({ socket: { data } }) with Object.prototype.timeout = 5000 ok throws Expected "onTimeout" callback to be a function ok
Bun.connect with no tls member, with Object.prototype.tls = true peer receives PLAINTEXT peer receives a TLS ClientHello peer receives PLAINTEXT

The four new cases in test/js/bun/net/socket.test.ts and test/js/bun/http/bun-serve-ssl.test.ts fail on 1.4.3-canary and pass on this branch, also after the merge with main (116dd64).

PR: #44824. It stays a draft until the self-review of the diff is done. Bun.SQL has a different cause and is a separate change. The Notes in the PR body say why.

@robobun

robobun commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 6:52 PM PT - Oct 15th, 2026

❌ @robobun, your commit 9cdfdaf has 2 failures in Build #126243 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 44824

That installs a local version of the PR into your bun-44824 executable, so you can run:

bun-44824 --bun

…genv2-dictionary-proto-pollution

# Conflicts:
#	src/codegen/bindgenv2/internal/dictionary.ts
#	test/js/bun/http/bun-serve-ssl.test.ts
#	test/js/bun/net/socket.test.ts
@robobun

robobun commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Closing: superseded by #45104.

#45104 replaced bindgen v1 and v2 with one generator and removed src/codegen/bindgenv2/, the directory this PR changes. The new Bun::Bindgen::DictionaryMemberReader::get stops the member lookup before Object.prototype, which is what this PR did in the old generator.

I ran the four test cases of this PR on a debug build of main (8ba82c3). All four pass. They fail on main before #45104 (116dd64).

Two of them check things that the new test in socket.test.ts does not: a handler on Object.prototype, and rejectUnauthorized for Bun.connect and Bun.serve. I can send those as a test-only PR if that is useful.

@robobun robobun closed this Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants