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

FATAL ERROR when setting 'onread' of a socket #37369

Open
aliclark opened this issue Feb 14, 2021 · 3 comments
Open

FATAL ERROR when setting 'onread' of a socket #37369

aliclark opened this issue Feb 14, 2021 · 3 comments
Labels
net Issues and PRs related to the net subsystem.

Comments

@aliclark
Copy link

aliclark commented Feb 14, 2021

What steps will reproduce the bug?

var net = require('net');
var socket = net.connect();
socket[Object.getOwnPropertySymbols(socket)[1]].onread = null;

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

Always

What is the expected behavior?

A Type Error should be thrown.

What do you see instead?

node[148008]: ../src/base_object-inl.h:177:static void node::BaseObject::InternalFieldSet(v8::Local<v8::String>, v8::Local<v8::Value>, const v8::PropertyCallbackInfo<void>&) [with int Field = 2; bool (v8::Value::* typecheck)() const = &v8::Value::IsFunction]: Assertion `((*value)->*typecheck)()' failed.
 1: 0xa03530 node::Abort() [node]
 2: 0xa035ae  [node]
 3: 0xad1fe2  [node]
 4: 0xf24a1e v8::internal::Object::SetPropertyWithAccessor(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::Maybe<v8::internal::ShouldThrow>) [node]
 5: 0xf44963 v8::internal::Object::SetPropertyInternal(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::Maybe<v8::internal::ShouldThrow>, v8::internal::StoreOrigin, bool*) [node]
 6: 0xf44f32 v8::internal::Object::SetProperty(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::StoreOrigin, v8::Maybe<v8::internal::ShouldThrow>) [node]
 7: 0xdd414f v8::internal::StoreIC::Store(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>, v8::internal::StoreOrigin) [node]
 8: 0xdd4a76 v8::internal::Runtime_StoreIC_Miss(int, unsigned long*, v8::internal::Isolate*) [node]
 9: 0x13ff179  [node]
Aborted (core dumped)

Additional information

Alternative POC:

var net = require('net');
net.createServer().listen()._handle.onread = 1;

Unexpectedly also crashes when assigning to a newly created object, sharing the 0th, 1st, or 2nd prototype.

var net = require('net');
var socket = net.connect();
var object = Object.create(Object.getPrototypeOf(socket[Object.getOwnPropertySymbols(socket)[1]]))
object.onread = null;
// also crashes:
//var object = Object.create(socket[Object.getOwnPropertySymbols(socket)[1]])
//var object = Object.create(Object.getPrototypeOf(Object.getPrototypeOf(socket[Object.getOwnPropertySymbols(socket)[1]])))
// does not crash:
//var object = Object.create(Object.getPrototypeOf(Object.getPrototypeOf(Object.getPrototypeOf(socket[Object.getOwnPropertySymbols(socket)[1]]))))

Alternative error:

var net = require('net');
var clone = require('clone');
clone(net.connect());
FATAL ERROR: v8::Object::SetInternalField() Internal field out of bounds
 1: 0xa03530 node::Abort() [node]
 2: 0x94e471 node::FatalError(char const*, char const*) [node]
 3: 0xb7745a v8::Utils::ReportApiFailure(char const*, char const*) [node]
 4: 0xb774e5  [node]
 5: 0xb8c5c4 v8::Object::SetInternalField(int, v8::Local<v8::Value>) [node]
 6: 0xf24a1e v8::internal::Object::SetPropertyWithAccessor(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::Maybe<v8::internal::ShouldThrow>) [node]
 7: 0xf44963 v8::internal::Object::SetPropertyInternal(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::Maybe<v8::internal::ShouldThrow>, v8::internal::StoreOrigin, bool*) [node]
 8: 0xf44f32 v8::internal::Object::SetProperty(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::StoreOrigin, v8::Maybe<v8::internal::ShouldThrow>) [node]
 9: 0x106e945 v8::internal::Runtime::SetObjectProperty(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::StoreOrigin, v8::Maybe<v8::internal::ShouldThrow>) [node]
10: 0x106f9e7 v8::internal::Runtime_SetKeyedProperty(int, unsigned long*, v8::internal::Isolate*) [node]
11: 0x13ff179  [node]
Aborted (core dumped)

The following issues are related:

@aliclark
Copy link
Author

aliclark commented Feb 14, 2021

There are 3 onread implementations I can see on the prototype chain, if it helps:

> Object.getPrototypeOf(Object.getPrototypeOf(socket[Object.getOwnPropertySymbols(socket)[1]])).onread
[Function: readStop]
> Object.getPrototypeOf(socket[Object.getOwnPropertySymbols(socket)[1]]).onread
[Function: listen]
> socket[Object.getOwnPropertySymbols(socket)[1]].onread
[Function: onStreamRead]

Of these only the attribute with value readStop is defined as an own property. The other 2 may be implemented by some kind of proxy magic?

@juanarbol juanarbol added net Issues and PRs related to the net subsystem. confirmed-bug Issues with confirmed bugs. labels Feb 15, 2021
@jasnell jasnell removed the confirmed-bug Issues with confirmed bugs. label Feb 22, 2021
@jasnell
Copy link
Member

jasnell commented Feb 22, 2021

We can't really consider this to be a bug. The example is monkey patching an internal function in a way that is simply not supported. The internal symbols (in this case the hidden kHandle symbol) and the object it references are both considered non-public Node.js internals.

What is the use case you're trying to achieve? It's possible we need an alternative public API to support the case.

@aliclark
Copy link
Author

aliclark commented Feb 22, 2021

Hi James, I'm sorry this wasn't clear - the bug isn't that kHandle is not modifiable. The bug is that upon being modified the result is the node.js process crashing abruptly, instead of throwing the TypeError exception which is what other properties do in this scenario.

The use-case is that in some cases, eg. the algorithm used by "clone" NPM package, assignment can be performed on objects without knowing whether the object is considered internal or non-internal.

Arguably any objects exposed to JavaScript should behave legally to JavaScript operations performed on them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants