Skip to content

buffer: fix silent data corruption with indices >= 2^32#62157

Closed
Felipeness wants to merge 1 commit intonodejs:mainfrom
Felipeness:fix/buffer-copy-large-indices
Closed

buffer: fix silent data corruption with indices >= 2^32#62157
Felipeness wants to merge 1 commit intonodejs:mainfrom
Felipeness:fix/buffer-copy-large-indices

Conversation

@Felipeness
Copy link

Buffer.copy() and Buffer.concat() silently produced incorrect results when operating with buffers whose size or copy offsets exceeded 2^32 bytes (4 GiB). This is a regression introduced in v22.7.0 by #54087.

Root cause

SlowCopy and FastCopy in src/node_buffer.cc used uint32_t for target_start, source_start, and to_copy parameters. Values exceeding 2^32 silently wrapped around due to integer overflow, causing memmove to write to the wrong position.

Changes

  • Change CopyImpl parameters from uint32_t to size_t
  • Change SlowCopy to use IntegerValue (int64_t) instead of As<Uint32>()->Value()
  • Add guard against negative int64_t values before casting to size_t
  • Change FastCopy to use uint64_t parameters and return type
  • Add pummel test covering: copy at 2^32 offset, boundary crossing at 2^32-1, and concat with total length > 2^32

Out of scope

Other functions in node_buffer.cc (CopyArrayBuffer, StringWrite, FastByteLengthUtf8) have similar uint32_t limitations but are not addressed here to keep the change focused on the reported regression. These could be fixed in follow-up PRs.

Test plan

  • Buffer.copy() with targetStart >= 2^32 writes to the correct position
  • Buffer.copy() crossing the 2^32-1 boundary writes correctly
  • Buffer.concat() with total result length > 2^32 preserves all data
  • Test is in test/pummel/ and skips on 32-bit platforms and low-memory environments

Fixes: #55422

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 8, 2026
SlowCopy and FastCopy in node_buffer.cc used uint32_t for target_start,
source_start, and to_copy parameters. When these values exceeded 2^32,
they silently wrapped around, causing Buffer.copy and Buffer.concat to
produce incorrect results with large buffers.

Change CopyImpl parameters from uint32_t to size_t, SlowCopy to use
IntegerValue (int64_t) instead of Uint32, and FastCopy to use uint64_t.
Add a guard against negative int64_t values before casting to size_t
to prevent potential out-of-bounds access.

Note: other functions in node_buffer.cc (CopyArrayBuffer, StringWrite,
FastByteLengthUtf8) have similar uint32_t limitations but are not
addressed here to keep this change focused on the reported regression.

Fixes: nodejs#55422
@Felipeness Felipeness force-pushed the fix/buffer-copy-large-indices branch from 4a9feca to 4ab5e5a Compare March 8, 2026 19:54
@Renegade334
Copy link
Member

Duplicate of #61914.

@Felipeness
Copy link
Author

Thanks for the heads up! I wasn't aware of #61914 when opening this. Closing as duplicate — that PR covers the same fix.

One minor note for #61914: the test allocates 4GB+ buffers, so it might fit better in test/pummel/ rather than test/parallel/ to avoid CI timeouts on memory-constrained runners. But that's a detail for the existing PR to address.

@Felipeness Felipeness closed this Mar 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buffer.concat and Buffer.copy silently produce invalid results when the operation involves indices equal or greater than 2^32

3 participants