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 @MylesBorins
trevnorris authored and MylesBorins committed Oct 26, 2016
1 parent 19d6f06 commit 1230062
Show file tree
Hide file tree
Showing 23 changed files with 446 additions and 175 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
7 changes: 4 additions & 3 deletions src/fs_event_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,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());
CHECK_EQ(wrap->initialized_, false);
FSEventWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

if (args.Length() < 1 || !args[0]->IsString()) {
return env->ThrowTypeError("filename must be a valid string");
Expand Down Expand Up @@ -165,7 +165,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
9 changes: 6 additions & 3 deletions src/handle_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ 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->handle__);
Expand All @@ -28,7 +29,8 @@ void HandleWrap::Ref(const FunctionCallbackInfo<Value>& args) {


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->handle__);
Expand All @@ -40,7 +42,8 @@ void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args) {
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 @@ -340,8 +340,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 @@ -370,8 +370,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 @@ -384,8 +384,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 @@ -411,8 +411,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 @@ -427,8 +427,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 @@ -690,7 +690,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 1230062

Please sign in to comment.