Skip to content

fix: validate native ArrayBuffer sizes - #1642

Open
huytdps13400 wants to merge 1 commit into
margelo:mainfrom
huytdps13400:fix/1545-array-buffer-size
Open

fix: validate native ArrayBuffer sizes#1642
huytdps13400 wants to merge 1 commit into
margelo:mainfrom
huytdps13400:fix/1545-array-buffer-size

Conversation

@huytdps13400

Copy link
Copy Markdown

Summary

  • reject non-finite, negative, and out-of-range native ArrayBuffer sizes before converting the JavaScript number to size_t
  • use the exclusive 2^size_t_bits bound so the 64-bit conversion boundary cannot be admitted through double rounding
  • cover NaN, Infinity, negative, and Number.MAX_VALUE inputs in the shared Android runtime harness

Breaking changes

None. Valid non-negative sizes keep the existing conversion and allocation behavior. Invalid values now throw deterministically instead of reaching an undefined conversion or allocation attempt.

Verification

  • RED on Android API 37 before the fix: both C++ and Swift/Kotlin harness variants returned an ArrayBuffer for NaN instead of throwing
  • GREEN Android harness: 8/8 invalid-size assertions passed across both variants
  • focused createNativeArrayBuffer harness: 12/12 passed, including valid allocation and external-memory reporting
  • Android arm64-v8a debug build passed
  • bun run build
  • bun typecheck
  • bun specs, with no generated diff
  • example TypeScript lint passed
  • C++ and Swift format scripts passed through the Xcode toolchain
  • full lint-all reached Kotlin lint but the local machine does not have the ktlint executable; no Kotlin source is changed and CI will run that lane on the configured runner

AI assistance

Codex using GPT-5.6 Sol assisted with reproduction, implementation, tests, and review.

Fixes #1545

@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
nitro-docs Skipped Skipped Sep 7, 2026 11:09pm UTC

Request Review

Comment on lines 82 to 92
std::shared_ptr<ArrayBuffer> HybridNitroModulesProxy::createNativeArrayBuffer(double size) {
const double maximumSizeExclusive = std::ldexp(1.0, std::numeric_limits<size_t>::digits);
if (!std::isfinite(size) || size < 0 || size >= maximumSizeExclusive) [[unlikely]] {
throw std::invalid_argument("ArrayBuffer size must be finite, non-negative, and within the platform size limit.");
}
return ArrayBuffer::allocate(static_cast<size_t>(size));
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why here instead of inside ArrayBuffer::allocate(..) directly? Also not sure if we should even catch this, I mean allocating NaN buffers is honestly a user mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

createNativeArrayBuffer accepts invalid allocation sizes

2 participants