Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffer.utf8Write() fails to write when buffer length exceeds 2 GB #51817

Open
adembo opened this issue Feb 21, 2024 · 1 comment · May be fixed by #51821
Open

Buffer.utf8Write() fails to write when buffer length exceeds 2 GB #51817

adembo opened this issue Feb 21, 2024 · 1 comment · May be fixed by #51821
Labels
buffer Issues and PRs related to the buffer subsystem. confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors.

Comments

@adembo
Copy link

adembo commented Feb 21, 2024

Version

18.16.0, 21.6.2

Platform

Darwin LX2970YFV4 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:30:44 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

Welcome to Node.js v18.16.0.
Type ".help" for more information.
> b = Buffer.allocUnsafeSlow(2 ** 32)
<Buffer 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... 4294967246 more bytes>
> b.utf8Write('a', 0, 0x100000000)
1
> b.utf8Write('a', 1, 0x100000000)
1
> b.utf8Write('a', 2, 0x100000000)
0
> b.utf8Write('a', 3, 0x100000000)
0
> b.utf8Write('a', 4, 0x100000000)
0

How often does it reproduce? Is there a required condition?

All the time.

What is the expected behavior? Why is that the expected behavior?

The utf8Write() function call should succeed and actually write something.

What do you see instead?

See reproduction steps.

Additional information

I built NodeJS from source code and used lldb and source code inspection to understand what's happening here.

If I allocate a buffer with size 2^32, an offset of 2, and provide a maxLength of is 2^32, eventually there's an attempt to pass 2^32-2 from the node::StringBytes::Write() buflen argument (size_t) into the v8::String::WriteUtf8() capacity argument (int). This cast causes an integer overflow and the v8 function ends up with a capacity argument of -2. This in turn causes the rest of v8::String::WriteUtf8() to short-circuit and not write anything (release builds) or crash in a DHECK_GE (debug builds).

Here's the top of the stack trace from the DCHECK_GE crash:

#
# Fatal error in ../deps/v8/src/api/api.cc, line 5626
# Debug check failed: remaining_capacity >= 0 (-2 vs. 0).
#
#
#
#FailureMessage Object: 0x16f33ec58
 1: 0x100b34cbc node::DumpBacktrace(__sFILE*) [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
 2: 0x100d52238 node::NodePlatform::GetStackTracePrinter()::$_3::operator()() const [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
 3: 0x100d521f4 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
 4: 0x102798ea8 V8_Fatal(char const*, int, char const*, ...) [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
 5: 0x102798a44 std::__1::enable_if<!std::is_function<std::__1::remove_pointer<char>::type>::value && !std::is_enum<char>::value && has_output_operator<char, v8::base::CheckMessageStream>::value, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>::type v8::base::PrintCheckOperand<char>(char) [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
 6: 0x100fb6868 v8::String::WriteUtf8(v8::Isolate*, char*, int, int*, int) const [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
 7: 0x100e0b77c node::StringBytes::Write(v8::Isolate*, char*, unsigned long, v8::Local<v8::Value>, node::encoding) [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
 8: 0x100c10be4 void node::Buffer::(anonymous namespace)::StringWrite<(node::encoding)1>(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/adar.lieber-dembo/h/source/node/out/Debug/node]
@kvakil kvakil added confirmed-bug Issues with confirmed bugs. buffer Issues and PRs related to the buffer subsystem. good first issue Issues that are suitable for first-time contributors. labels Feb 21, 2024
@kylo5aby
Copy link
Contributor

Since the limitation of string utf8 length within the range of int in V8, I believe that if maxLength overflows the int range, restricting maxLength to the int range would solve the issue. If this perspective is reasonable, I'm willing to submit a PR.

@kylo5aby kylo5aby linked a pull request Feb 21, 2024 that will close this issue
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. confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants