src: remove TOCTOU race condition when encoding SAB-backed `Buffer`s · nodejs/node@85d1639 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 638
• Pull requests 759
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 85d1639

Browse filesBrowse the repository at this point in the historyBrowse files

aduh95

authored and

sxa

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 7cead61 commit 85d1639
Copy full SHA for 85d1639

2 files changed

+32-8Lines changed: 32 additions & 8 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• src
  • encoding_binding.cc
  • node_buffer.cc

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎src/encoding_binding.cc‎

Copy file name to clipboardExpand all lines: src/encoding_binding.cc

+14-2Lines changed: 14 additions & 2 deletions

Original file line number	Diff line number	Diff line change
@@ -152,15 +152,20 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo<Value>& args) {
152	152	Environment* env = Environment::GetCurrent(args); // list, flags
153	153
154	154	CHECK_GE(args.Length(), 1);
	155	+ auto isShared = args[0]->IsSharedArrayBuffer();
155	156
156		- if (!(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer() ||
157		- args[0]->IsArrayBufferView())) {
	157	+ if (!(args[0]->IsArrayBuffer() || isShared || args[0]->IsArrayBufferView())) {
158	158	return node::THROW_ERR_INVALID_ARG_TYPE(
159	159	env->isolate(),
160	160	"The \"list\" argument must be an instance of SharedArrayBuffer, "
161	161	"ArrayBuffer or ArrayBufferView.");
162	162	}
163	163
	164	+ if (args[0]->IsArrayBufferView()) {
	165	+ Local<v8::ArrayBufferView> view = args[0].As<v8::ArrayBufferView>();
	166	+ isShared = view->Buffer()->IsSharedArrayBuffer();
	167	+ }
	168	+
164	169	ArrayBufferViewContents<char> buffer(args[0]);
165	170
166	171	bool ignore_bom = args[1]->IsTrue();
@@ -169,6 +174,13 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo<Value>& args) {
169	174	const char* data = buffer.data();
170	175	size_t length = buffer.length();
171	176
	177	+ std::unique_ptr<char[]> data_copy;
	178	+ if (isShared && length != 0) {
	179	+ data_copy = std::make_unique_for_overwrite<char[]>(length);
	180	+ memcpy(data_copy.get(), data, length);
	181	+ data = data_copy.get();
	182	+ }
	183	+
172	184	if (!ignore_bom && length >= 3) {
173	185	if (memcmp(data, "\xEF\xBB\xBF", 3) == 0) {
174	186	data += 3;

Collapse file

‎src/node_buffer.cc‎

Copy file name to clipboardExpand all lines: src/node_buffer.cc

+18-6Lines changed: 18 additions & 6 deletions

Original file line number	Diff line number	Diff line change
@@ -567,19 +567,31 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
567	567	THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
568	568	ArrayBufferViewContents<char> buffer(args[0]);
569	569
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();
572	576
573	577	size_t start = 0;
574	578	size_t end = 0;
575	579	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));
579	583	size_t length = end - start;
580	584
	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	+
581	593	Local<Value> ret;
582		- if (StringBytes::Encode(isolate, buffer.data() + start, length, encoding)
	594	+ if (StringBytes::Encode(isolate, data_ptr + start, length, encoding)
583	595	.ToLocal(&ret)) {
584	596	args.GetReturnValue().Set(ret);
585	597	}

0 commit comments

Comments

0 (0)