Skip to content

Commit

Permalink
src: no abort from getter if object isn't wrapped
Browse files Browse the repository at this point in the history 
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 <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
@trevnorris @rvagg
trevnorris authored and rvagg committed Jun 2, 2016
1 parent 0d08fc4 commit 4333fda
Show file tree
Hide file tree
Showing 23 changed files with 462 additions and 178 deletions.
4 changes: 4 additions & 0 deletions src/base-object-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ inline BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> handle)
: handle_(env->isolate(), handle),
env_(env) {
CHECK_EQ(false, handle.IsEmpty());
// The zero field holds a pointer to the handle. Immediately set it to
// nullptr in case it's accessed by the user before construction is complete.
if (handle->InternalFieldCount() > 0)
handle->SetAlignedPointerInInternalField(0, nullptr);
}


Expand Down
6 changes: 4 additions & 2 deletions src/fs_event_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ void FSEventWrap::New(const FunctionCallbackInfo<Value>& args) {
void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

FSEventWrap* wrap = Unwrap<FSEventWrap>(args.Holder());
FSEventWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

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


void FSEventWrap::Close(const FunctionCallbackInfo<Value>& args) {
FSEventWrap* wrap = Unwrap<FSEventWrap>(args.Holder());
FSEventWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

if (wrap == nullptr || wrap->initialized_ == false)
return;
Expand Down
12 changes: 8 additions & 4 deletions src/handle_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,35 @@ using v8::Value;


void HandleWrap::Ref(const FunctionCallbackInfo<Value>& args) {
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
HandleWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

if (IsAlive(wrap))
uv_ref(wrap->GetHandle());
}


void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args) {
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
HandleWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

if (IsAlive(wrap))
uv_unref(wrap->GetHandle());
}


void HandleWrap::HasRef(const FunctionCallbackInfo<Value>& args) {
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
HandleWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
args.GetReturnValue().Set(HasRef(wrap));
}


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

HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
HandleWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

// Guard against uninitialized handle or double close.
if (!IsAlive(wrap))
Expand Down
23 changes: 16 additions & 7 deletions src/js_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ static void FreeCallback(char* data, void* hint) {


void JSStream::DoAlloc(const FunctionCallbackInfo<Value>& args) {
JSStream* wrap = Unwrap<JSStream>(args.Holder());
JSStream* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

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


void JSStream::DoRead(const FunctionCallbackInfo<Value>& args) {
JSStream* wrap = Unwrap<JSStream>(args.Holder());
JSStream* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

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


void JSStream::DoAfterWrite(const FunctionCallbackInfo<Value>& args) {
JSStream* wrap = Unwrap<JSStream>(args.Holder());
WriteWrap* w = Unwrap<WriteWrap>(args[0].As<Object>());
JSStream* wrap;
CHECK(args[0]->IsObject());
WriteWrap* w;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
ASSIGN_OR_RETURN_UNWRAP(&w, args[0].As<Object>());

wrap->OnAfterWrite(w);
}


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

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


void JSStream::ReadBuffer(const FunctionCallbackInfo<Value>& args) {
JSStream* wrap = Unwrap<JSStream>(args.Holder());
JSStream* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

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


void JSStream::EmitEOF(const FunctionCallbackInfo<Value>& args) {
JSStream* wrap = Unwrap<JSStream>(args.Holder());
JSStream* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

wrap->OnRead(UV_EOF, nullptr);
}
Expand Down
23 changes: 12 additions & 11 deletions src/node_contextify.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ class ContextifyContext {
static void GlobalPropertyGetterCallback(
Local<Name> property,
const PropertyCallbackInfo<Value>& args) {
ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());
ContextifyContext* ctx;
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());

// Stil initializing
if (ctx->context_.IsEmpty())
Expand Down Expand Up @@ -373,8 +373,8 @@ class ContextifyContext {
Local<Name> property,
Local<Value> value,
const PropertyCallbackInfo<Value>& args) {
ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());
ContextifyContext* ctx;
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());

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

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

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

static void GlobalPropertyEnumeratorCallback(
const PropertyCallbackInfo<Array>& args) {
ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());
ContextifyContext* ctx;
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Data().As<Object>());

// Stil initializing
if (ctx->context_.IsEmpty())
Expand Down Expand Up @@ -806,7 +806,8 @@ class ContextifyScript : public BaseObject {
return false;
}

ContextifyScript* wrapped_script = Unwrap<ContextifyScript>(args.Holder());
ContextifyScript* wrapped_script;
ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder(), false);
Local<UnboundScript> unbound_script =
PersistentToLocal(env->isolate(), wrapped_script->script_);
Local<Script> script = unbound_script->BindToCurrentContext();
Expand Down
Loading

0 comments on commit 4333fda

Please sign in to comment.