| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
|
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. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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
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
Fixes: #55422