Assign process.debugPort with some value may results in an abort

[zyscoder]

• Version: v14.15.1
• Platform: Linux 5.8.0-38-generic The binary and long term compatibility with node #43~20.04.1-Ubuntu SMP Tue Jan 12 16:39:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: process

What steps will reproduce the bug?

Setup a node instance,

» node

and run the following javascript code.

process.debugPort = 772189652114705600000
//value -1 works too: process.debugPort = -1
process.on('str',function(){})

Then the node instance occurs an abort.

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

This abort can always be triggered following the steps above.

What is the expected behavior?

Maybe some check for the assignement of the process.debugPort is necessary. If any error occurs, an exception or other similar error-reporting stuff should be thrown. There is no reason to abort the whole node process.

What do you see instead?

» node
Welcome to Node.js v14.15.1.
Type ".help" for more information.
> 
> process.debugPort = 772189652114705600000
772189652114705600000
> processnode[209909]: ../src/node_options.h:33:int node::HostPort::port() const: Assertion `(port_) >= (0)' failed.
 1: 0xa03530 node::Abort() [node]
 2: 0xa035ae  [node]
 3: 0xa7a6b2  [node]
 4: 0xf427db v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*) [node]
 5: 0xf42398  [node]
 6: 0xee5c66 v8::internal::JSReceiver::GetOwnPropertyDescriptor(v8::internal::LookupIterator*, v8::internal::PropertyDescriptor*) [node]
 7: 0xee60e3 v8::internal::JSReceiver::GetOwnPropertyDescriptor(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyDescriptor*) [node]
 8: 0x107197b v8::internal::Runtime_GetOwnPropertyDescriptor(int, unsigned long*, v8::internal::Isolate*) [node]
 9: 0x13ff179  [node]
[1]    209909 abort (core dumped)  node                                                                                                                                                                                            

Additional information

[aduh95]

Can reproduce on master. Interestingly, it doesn't happen if we eval the code, or put it into a file:

$ out/Release/node -e 'process.debugPort=-1;process.on("str",()=>{})'
$ echo 'process.debugPort=-1;process.on("str",()=>{})' | out/Release/node
$ out/Release/node
Welcome to Node.js v16.0.0-pre.
Type ".help" for more information.
> process.debugPort=-1;process.on("str",()=>{})
out/Release/node[59141]: ../src/node_options.h:33:int node::HostPort::port() const: Assertion `(port_) >= (0)' failed.
 1: 0x1070bd895 node::Abort() […/out/Release/node]
 2: 0x1070bd701 node::Assert(node::AssertionInfo const&) […/out/Release/node]
 3: 0x10712e668 node::DebugPortGetter(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&) […/out/Release/node]
 4: 0x1074be86e v8::internal::PropertyCallbackArguments::BasicCallNamedGetterCallback(void (*)(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&), v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) […/out/Release/node]
 5: 0x10763a5e0 v8::internal::PropertyCallbackArguments::CallAccessorGetter(v8::internal::Handle<v8::internal::AccessorInfo>, v8::internal::Handle<v8::internal::Name>) […/out/Release/node]
 6: 0x107639573 v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*) […/out/Release/node]
 7: 0x107638b9d v8::internal::Object::GetProperty(v8::internal::LookupIterator*, bool) […/out/Release/node]
 8: 0x1075eb7a7 v8::internal::JSReceiver::GetOwnPropertyDescriptor(v8::internal::LookupIterator*, v8::internal::PropertyDescriptor*) […/out/Release/node]
 9: 0x1075e7eb6 v8::internal::JSReceiver::GetOwnPropertyDescriptor(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyDescriptor*) […/out/Release/node]
10: 0x10778739a v8::internal::Runtime_GetOwnPropertyDescriptor(int, unsigned long*, v8::internal::Isolate*) […/out/Release/node]
11: 0x107b1a8b9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit […/out/Release/node]
12: 0x107b03293 Builtins_ObjectGetOwnPropertyDescriptor […/out/Release/node]
13: 0x107ab53e5 Builtins_InterpreterEntryTrampoline […/out/Release/node]
[2]    59141 abort      out/Release/node
$ out/Release/node -p 'process.debugPort=-1;process.on("str",()=>{})' 
out/Release/node[59151]: ../src/node_options.h:33:int node::HostPort::port() const: Assertion `(port_) >= (0)' failed.
 1: 0x1078e5895 node::Abort() […/out/Release/node]
 2: 0x1078e5701 node::Assert(node::AssertionInfo const&) […/out/Release/node]
 3: 0x107956668 node::DebugPortGetter(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&) […/out/Release/node]
 4: 0x107ce686e v8::internal::PropertyCallbackArguments::BasicCallNamedGetterCallback(void (*)(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&), v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) […/out/Release/node]
 5: 0x107e625e0 v8::internal::PropertyCallbackArguments::CallAccessorGetter(v8::internal::Handle<v8::internal::AccessorInfo>, v8::internal::Handle<v8::internal::Name>) […/out/Release/node]
 6: 0x107e61573 v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*) […/out/Release/node]
 7: 0x107e60b9d v8::internal::Object::GetProperty(v8::internal::LookupIterator*, bool) […/out/Release/node]
 8: 0x107e137a7 v8::internal::JSReceiver::GetOwnPropertyDescriptor(v8::internal::LookupIterator*, v8::internal::PropertyDescriptor*) […/out/Release/node]
 9: 0x107e0feb6 v8::internal::JSReceiver::GetOwnPropertyDescriptor(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyDescriptor*) […/out/Release/node]
10: 0x107faf39a v8::internal::Runtime_GetOwnPropertyDescriptor(int, unsigned long*, v8::internal::Isolate*) […/out/Release/node]
11: 0x1083428b9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit […/out/Release/node]
12: 0x10832b293 Builtins_ObjectGetOwnPropertyDescriptor […/out/Release/node]
13: 0x1082dd3e5 Builtins_InterpreterEntryTrampoline […/out/Release/node]
[2]    59151 abort      out/Release/node -p 'process.debugPort=-1;process.on("str",()=>{})'

[Ayase-252]

node/src/node_process_object.cc

Line 199 in e79471d

env->owns_process_state() ? DebugPortSetter : nullptr,

It seems that assignment to process.debugPort only makes sense when env.own_process_state() is true. That is

node/src/env-inl.h

Lines 807 to 809 in e79471d

	inline bool Environment::owns_process_state() const {
	return flags_ & EnvironmentFlags::kOwnsProcessState;
	}

Could it be an valid fix that adding a CHECK to assert debugPort is within the range of int32 in DebugPortSetter?

node/src/node_process_object.cc

Lines 58 to 65 in e79471d

	static void DebugPortSetter(Local<Name> property,
	Local<Value> value,
	const PropertyCallbackInfo<void>& info) {
	Environment* env = Environment::GetCurrent(info);
	int32_t port = value->Int32Value(env->context()).FromMaybe(0);
	ExclusiveAccess<HostPort>::Scoped host_port(env->inspector_host_port());
	host_port->set_port(static_cast<int>(port));
	}