fix: ctype UB, WebSocket over-read, stoi crash, signed shift UB, proto pollution

[VirajMishra1]

fix: ctype UB, WebSocket over-read, stoi crash, shift UB, proto pollution

Six correctness and safety fixes across four files:

1. src/inspector_io.cc:381 -- isdigit called on plain char (undefined behavior)

::isdigit is defined only for values representable as unsigned char or EOF.
Passing a plain char (which may be negative on platforms where char is signed)
is undefined behavior. Fixed with an (unsigned char) cast in the lambda.

2. src/inspector_socket.cc:363 -- WebSocket extended-frame buffer over-read

The extended-payload-length bounds check used buffer.size() - kMaskingKeyWidthInBytes
as the minimum remaining bytes, but buffer is the full original buffer while
it may already have advanced into it. On a crafted extended-length frame,
payload_length bytes could be read past the end of the available data.
Fixed to check buffer.end() - it (remaining bytes from current position).

3. src/inspector_io.cc:383 -- std::stoi throws on out-of-range port strings

std::stoi throws std::out_of_range for digit-only strings that exceed INT_MAX
(e.g. "99999999999" passes the isdigit check but crashes). Added
try/catch (std::exception&) to handle both invalid_argument and out_of_range.

4. src/inspector_io.cc:386 -- signed left shift is undefined behavior

target_session_id << 16 on a signed int is UB if the result overflows.
Fixed with static_cast<unsigned int>(target_session_id) << 16.

5. lib/_http_outgoing.js:287 -- prototype-inheriting header dict

const headers = {} in _renderHeaders() creates an object that inherits
from Object.prototype. Header names are user-controlled strings; a header named
__proto__ or toString can shadow prototype properties.
Fixed to { __proto__: null }.

6. lib/os.js:219 -- prototype-inheriting network interface dict

Same issue in networkInterfaces(): the result object is keyed by OS interface
names. Fixed to { __proto__: null }.

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/inspector
• @nodejs/net

[MikeMcC399]

Caution

AgentScan found account activity patterns that may be consistent with
automation. This is a heuristic, not proof that this pull request was
opened by an agent or violates policy. AI-assisted contributions are
permitted, but automated tooling must not open pull requests without
advance approval, and contributors must personally understand, test,
verify, and take responsibility for every submitted change. See the
AgentScan analysis,
AI use policy,
and
automation policy
for additional context.

[MikeMcC399]

Please make sure you have read and understood the following documents:

• Guides and FAQs for first-time contributors
• contribution and automation policies
• pull request guide
• AI use policy
• Code of Conduct

[VirajMishra1]

I have read and understood all of the linked documents -- the contributing guide, pull request guide, AI use policy, automation policy, and Code of Conduct.

To be transparent: I used AI tooling to help identify these bugs and draft the initial code, but I personally reviewed each change, understand what it does, and take responsibility for it. I did not use automated tooling to open this PR -- I opened it manually after reviewing the diff. Happy to discuss any of the specific changes if that would help.

[MikeMcC399]

Please check the CI failures for your PR. You can view them under

https://github.com/nodejs/node/actions?query=actor%3AVirajMishra1+is%3Afailure

You should make sure that your changes run successfully locally. See https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#step-6-test

[VirajMishra1]

Thanks for checking. Two CI failures, both now fixed:

1. Build failures (inspector_io.cc): The try/catch around std::stoi doesn't compile when -fno-exceptions is set. Replaced with std::from_chars (C++17, no exceptions needed, same overflow-safe behavior).

2. Commit message lint: The title used fix: which isn't a valid Node.js subsystem prefix. Amended to inspector,http,os: fix ctype UB, stoi overflow, proto pollution (within 72 chars, valid subsystems, with Signed-off-by trailer).

Force-pushed the updated commit. CI should be clean now.

[panva]

this alone would likely need a full http CI benchmark

[VirajMishra1]

I reverted it for now, happy to revisit in a separate PR with benchmarks.

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (6e7818e) to head (87a5ab0).
⚠️ Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
src/inspector_io.cc	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65763      +/-   ##
==========================================
+ Coverage   89.95%   90.05%   +0.09%     
==========================================
  Files         757      769      +12     
  Lines      258053   261322    +3269     
  Branches    48934    49631     +697     
==========================================
+ Hits       232144   235336    +3192     
- Misses      16962    17035      +73     
- Partials     8947     8951       +4     

Files with missing lines	Coverage Δ
src/inspector_socket.cc	82.90% <100.00%> (+0.04%)	⬆️
src/inspector_io.cc	91.16% <85.71%> (-0.64%)	⬇️

... and 74 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[MikeMcC399]

The description in the body of the PR and the details in the title of the PR no longer correspond to the contents of the attached commit.