Skip to content

Commit f043013

Browse files
committed
src: remove TOCTOU race condition when encoding SAB-backed Buffers
Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #63517 Refs: https://hackerone.com/reports/3752489 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 8e75efb commit f043013

2 files changed

Lines changed: 32 additions & 8 deletions

File tree

src/encoding_binding.cc

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,15 +418,20 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo<Value>& args) {
418418
Environment* env = Environment::GetCurrent(args); // list, flags
419419

420420
CHECK_GE(args.Length(), 1);
421+
auto isShared = args[0]->IsSharedArrayBuffer();
421422

422-
if (!(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer() ||
423-
args[0]->IsArrayBufferView())) {
423+
if (!(args[0]->IsArrayBuffer() || isShared || args[0]->IsArrayBufferView())) {
424424
return node::THROW_ERR_INVALID_ARG_TYPE(
425425
env->isolate(),
426426
"The \"list\" argument must be an instance of SharedArrayBuffer, "
427427
"ArrayBuffer or ArrayBufferView.");
428428
}
429429

430+
if (args[0]->IsArrayBufferView()) {
431+
Local<v8::ArrayBufferView> view = args[0].As<v8::ArrayBufferView>();
432+
isShared = view->Buffer()->IsSharedArrayBuffer();
433+
}
434+
430435
ArrayBufferViewContents<char> buffer(args[0]);
431436

432437
bool ignore_bom = args[1]->IsTrue();
@@ -435,6 +440,13 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo<Value>& args) {
435440
const char* data = buffer.data();
436441
size_t length = buffer.length();
437442

443+
std::unique_ptr<char[]> data_copy;
444+
if (isShared && length != 0) {
445+
data_copy = std::make_unique_for_overwrite<char[]>(length);
446+
memcpy(data_copy.get(), data, length);
447+
data = data_copy.get();
448+
}
449+
438450
if (!ignore_bom && length >= 3) {
439451
if (memcmp(data, "\xEF\xBB\xBF", 3) == 0) {
440452
data += 3;

src/node_buffer.cc

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -567,19 +567,31 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
567567
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
568568
ArrayBufferViewContents<char> buffer(args[0]);
569569

570-
if (buffer.length() == 0)
571-
return args.GetReturnValue().SetEmptyString();
570+
auto buffer_length = buffer.length();
571+
const char* data_ptr = buffer.data();
572+
573+
Local<ArrayBufferView> view = args[0].As<ArrayBufferView>();
574+
575+
if (buffer_length == 0) return args.GetReturnValue().SetEmptyString();
572576

573577
size_t start = 0;
574578
size_t end = 0;
575579
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[1], 0, &start));
576-
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], buffer.length(), &end));
577-
if (end < start) end = start;
578-
THROW_AND_RETURN_IF_OOB(Just(end <= buffer.length()));
580+
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], buffer_length, &end));
581+
if (end <= start) return args.GetReturnValue().SetEmptyString();
582+
THROW_AND_RETURN_IF_OOB(Just(end <= buffer_length));
579583
size_t length = end - start;
580584

585+
std::unique_ptr<char[]> data_copy;
586+
if (view->Buffer()->IsSharedArrayBuffer()) {
587+
data_copy = std::make_unique_for_overwrite<char[]>(length);
588+
memcpy(data_copy.get(), data_ptr + start, length);
589+
data_ptr = data_copy.get();
590+
start = 0;
591+
}
592+
581593
Local<Value> ret;
582-
if (StringBytes::Encode(isolate, buffer.data() + start, length, encoding)
594+
if (StringBytes::Encode(isolate, data_ptr + start, length, encoding)
583595
.ToLocal(&ret)) {
584596
args.GetReturnValue().Set(ret);
585597
}

0 commit comments

Comments
 (0)