Skip to content

Commit c0e6c66

Browse files
committed
src: no abort from getter if object isn't wrapped
v8::Object::GetAlignedPointerFromInternalField() returns a random value if Wrap() hasn't been run on the object handle. Causing v8 to abort if certain getters are accessed. It's possible to access these getters and functions during class construction through the AsyncWrap init() callback, and also possible in a subset of those scenarios while running the persistent handle visitor. Mitigate this issue by manually setting the internal aligned pointer field to nullptr in the BaseObject constructor and add necessary logic to return appropriate values when nullptr is encountered. PR-URL: #6184 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 13e5d4f commit c0e6c66

23 files changed

+462
-178
lines changed

src/base-object-inl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ inline BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> handle)
1414
: handle_(env->isolate(), handle),
1515
env_(env) {
1616
CHECK_EQ(false, handle.IsEmpty());
17+
// The zero field holds a pointer to the handle. Immediately set it to
18+
// nullptr in case it's accessed by the user before construction is complete.
19+
if (handle->InternalFieldCount() > 0)
20+
handle->SetAlignedPointerInInternalField(0, nullptr);
1721
}
1822

1923

src/fs_event_wrap.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ void FSEventWrap::New(const FunctionCallbackInfo<Value>& args) {
8686
void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) {
8787
Environment* env = Environment::GetCurrent(args);
8888

89-
FSEventWrap* wrap = Unwrap<FSEventWrap>(args.Holder());
89+
FSEventWrap* wrap;
90+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
9091

9192
static const char kErrMsg[] = "filename must be a string or Buffer";
9293
if (args.Length() < 1)
@@ -181,7 +182,8 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename,
181182

182183

183184
void FSEventWrap::Close(const FunctionCallbackInfo<Value>& args) {
184-
FSEventWrap* wrap = Unwrap<FSEventWrap>(args.Holder());
185+
FSEventWrap* wrap;
186+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
185187

186188
if (wrap == nullptr || wrap->initialized_ == false)
187189
return;

src/handle_wrap.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,35 @@ using v8::Value;
1818

1919

2020
void HandleWrap::Ref(const FunctionCallbackInfo<Value>& args) {
21-
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
21+
HandleWrap* wrap;
22+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
2223

2324
if (IsAlive(wrap))
2425
uv_ref(wrap->GetHandle());
2526
}
2627

2728

2829
void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args) {
29-
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
30+
HandleWrap* wrap;
31+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
3032

3133
if (IsAlive(wrap))
3234
uv_unref(wrap->GetHandle());
3335
}
3436

3537

3638
void HandleWrap::HasRef(const FunctionCallbackInfo<Value>& args) {
37-
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
39+
HandleWrap* wrap;
40+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
3841
args.GetReturnValue().Set(HasRef(wrap));
3942
}
4043

4144

4245
void HandleWrap::Close(const FunctionCallbackInfo<Value>& args) {
4346
Environment* env = Environment::GetCurrent(args);
4447

45-
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
48+
HandleWrap* wrap;
49+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
4650

4751
// Guard against uninitialized handle or double close.
4852
if (!IsAlive(wrap))

src/js_stream.cc

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ static void FreeCallback(char* data, void* hint) {
135135

136136

137137
void JSStream::DoAlloc(const FunctionCallbackInfo<Value>& args) {
138-
JSStream* wrap = Unwrap<JSStream>(args.Holder());
138+
JSStream* wrap;
139+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
139140

140141
uv_buf_t buf;
141142
wrap->OnAlloc(args[0]->Int32Value(), &buf);
@@ -150,7 +151,8 @@ void JSStream::DoAlloc(const FunctionCallbackInfo<Value>& args) {
150151

151152

152153
void JSStream::DoRead(const FunctionCallbackInfo<Value>& args) {
153-
JSStream* wrap = Unwrap<JSStream>(args.Holder());
154+
JSStream* wrap;
155+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
154156

155157
CHECK(Buffer::HasInstance(args[1]));
156158
uv_buf_t buf = uv_buf_init(Buffer::Data(args[1]), Buffer::Length(args[1]));
@@ -159,23 +161,29 @@ void JSStream::DoRead(const FunctionCallbackInfo<Value>& args) {
159161

160162

161163
void JSStream::DoAfterWrite(const FunctionCallbackInfo<Value>& args) {
162-
JSStream* wrap = Unwrap<JSStream>(args.Holder());
163-
WriteWrap* w = Unwrap<WriteWrap>(args[0].As<Object>());
164+
JSStream* wrap;
165+
CHECK(args[0]->IsObject());
166+
WriteWrap* w;
167+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
168+
ASSIGN_OR_RETURN_UNWRAP(&w, args[0].As<Object>());
164169

165170
wrap->OnAfterWrite(w);
166171
}
167172

168173

169174
template <class Wrap>
170175
void JSStream::Finish(const FunctionCallbackInfo<Value>& args) {
171-
Wrap* w = Unwrap<Wrap>(args[0].As<Object>());
176+
Wrap* w;
177+
CHECK(args[0]->IsObject());
178+
ASSIGN_OR_RETURN_UNWRAP(&w, args[0].As<Object>());
172179

173180
w->Done(args[1]->Int32Value());
174181
}
175182

176183

177184
void JSStream::ReadBuffer(const FunctionCallbackInfo<Value>& args) {
178-
JSStream* wrap = Unwrap<JSStream>(args.Holder());
185+
JSStream* wrap;
186+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
179187

180188
CHECK(Buffer::HasInstance(args[0]));
181189
char* data = Buffer::Data(args[0]);
@@ -197,7 +205,8 @@ void JSStream::ReadBuffer(const FunctionCallbackInfo<Value>& args) {
197205

198206

199207
void JSStream::EmitEOF(const FunctionCallbackInfo<Value>& args) {
200-
JSStream* wrap = Unwrap<JSStream>(args.Holder());
208+
JSStream* wrap;
209+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
201210

202211
wrap->OnRead(UV_EOF, nullptr);
203212
}

src/node_contextify.cc

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -343,8 +343,8 @@ class ContextifyContext {
343343
static void GlobalPropertyGetterCallback(
344344
Local<Name> property,
345345
const PropertyCallbackInfo<Value>& args) {
346-
ContextifyContext* ctx =
347-
Unwrap<ContextifyContext>(args.Data().As<Object>());
346+
ContextifyContext* ctx;
347+
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());
348348

349349
// Stil initializing
350350
if (ctx->context_.IsEmpty())
@@ -373,8 +373,8 @@ class ContextifyContext {
373373
Local<Name> property,
374374
Local<Value> value,
375375
const PropertyCallbackInfo<Value>& args) {
376-
ContextifyContext* ctx =
377-
Unwrap<ContextifyContext>(args.Data().As<Object>());
376+
ContextifyContext* ctx;
377+
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());
378378

379379
// Stil initializing
380380
if (ctx->context_.IsEmpty())
@@ -387,8 +387,8 @@ class ContextifyContext {
387387
static void GlobalPropertyQueryCallback(
388388
Local<Name> property,
389389
const PropertyCallbackInfo<Integer>& args) {
390-
ContextifyContext* ctx =
391-
Unwrap<ContextifyContext>(args.Data().As<Object>());
390+
ContextifyContext* ctx;
391+
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());
392392

393393
// Stil initializing
394394
if (ctx->context_.IsEmpty())
@@ -414,8 +414,8 @@ class ContextifyContext {
414414
static void GlobalPropertyDeleterCallback(
415415
Local<Name> property,
416416
const PropertyCallbackInfo<Boolean>& args) {
417-
ContextifyContext* ctx =
418-
Unwrap<ContextifyContext>(args.Data().As<Object>());
417+
ContextifyContext* ctx;
418+
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());
419419

420420
// Stil initializing
421421
if (ctx->context_.IsEmpty())
@@ -430,8 +430,8 @@ class ContextifyContext {
430430

431431
static void GlobalPropertyEnumeratorCallback(
432432
const PropertyCallbackInfo<Array>& args) {
433-
ContextifyContext* ctx =
434-
Unwrap<ContextifyContext>(args.Data().As<Object>());
433+
ContextifyContext* ctx;
434+
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());
435435

436436
// Stil initializing
437437
if (ctx->context_.IsEmpty())
@@ -806,7 +806,8 @@ class ContextifyScript : public BaseObject {
806806
return false;
807807
}
808808

809-
ContextifyScript* wrapped_script = Unwrap<ContextifyScript>(args.Holder());
809+
ContextifyScript* wrapped_script;
810+
ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder(), false);
810811
Local<UnboundScript> unbound_script =
811812
PersistentToLocal(env->isolate(), wrapped_script->script_);
812813
Local<Script> script = unbound_script->BindToCurrentContext();

0 commit comments

Comments
 (0)