Permalink
Browse files

n-api: avoid crash in napi_escape_scope()

V8 will crash if escape is called twice on the same
scope.

Add checks to avoid crashing if napi_escape_scope() is
called to try and do this.

Add test that tries to call napi_create_scope() twice.

PR-URL: #13651
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information...
mhdawson committed Jun 13, 2017
1 parent 9a655e9 commit 3e18c49657bcb0f2504def3d7d6d900f0c25e357
View
@@ -17,6 +17,7 @@
#include <vector>
#include "uv.h"
#include "node_api.h"
#include "node_internals.h"
#define NAPI_VERSION 1
@@ -156,14 +157,20 @@ class HandleScopeWrapper {
// across different versions.
class EscapableHandleScopeWrapper {
public:
explicit EscapableHandleScopeWrapper(v8::Isolate* isolate) : scope(isolate) {}
explicit EscapableHandleScopeWrapper(v8::Isolate* isolate)
: scope(isolate), escape_called_(false) {}
bool escape_called() const {
return escape_called_;
}
template <typename T>
v8::Local<T> Escape(v8::Local<T> handle) {
escape_called_ = true;
return scope.Escape(handle);
}
private:
v8::EscapableHandleScope scope;
bool escape_called_;
};
napi_handle_scope JsHandleScopeFromV8HandleScope(HandleScopeWrapper* s) {
@@ -718,7 +725,8 @@ const char* error_messages[] = {nullptr,
"An array was expected",
"Unknown failure",
"An exception is pending",
"The async work item was cancelled"};
"The async work item was cancelled",
"napi_escape_handle already called on scope"};
static napi_status napi_clear_last_error(napi_env env) {
CHECK_ENV(env);
@@ -746,10 +754,14 @@ napi_status napi_get_last_error_info(napi_env env,
CHECK_ENV(env);
CHECK_ARG(env, result);
// you must update this assert to reference the last message
// in the napi_status enum each time a new error message is added.
// We don't have a napi_status_last as this would result in an ABI
// change each time a message was added.
static_assert(
(sizeof (error_messages) / sizeof (*error_messages)) == napi_status_last,
node::arraysize(error_messages) == napi_escape_called_twice + 1,
"Count of error messages must match count of error values");
assert(env->last_error.error_code < napi_status_last);
assert(env->last_error.error_code <= napi_escape_called_twice);
// Wait until someone requests the last error information to fetch the error
// message string
@@ -2211,9 +2223,12 @@ napi_status napi_escape_handle(napi_env env,
v8impl::EscapableHandleScopeWrapper* s =
v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope);
*result = v8impl::JsValueFromV8LocalValue(
s->Escape(v8impl::V8LocalValueFromJsValue(escapee)));
return napi_clear_last_error(env);
if (!s->escape_called()) {
*result = v8impl::JsValueFromV8LocalValue(
s->Escape(v8impl::V8LocalValueFromJsValue(escapee)));
return napi_clear_last_error(env);
}
return napi_set_last_error(env, napi_escape_called_twice);
}
napi_status napi_new_instance(napi_env env,
View
@@ -67,7 +67,7 @@ typedef enum {
napi_generic_failure,
napi_pending_exception,
napi_cancelled,
napi_status_last
napi_escape_called_twice
} napi_status;
typedef napi_value (*napi_callback)(napi_env env,
@@ -10,6 +10,12 @@ testHandleScope.NewScope();
assert.ok(testHandleScope.NewScopeEscape() instanceof Object);
assert.throws(
() => {
testHandleScope.NewScopeEscapeTwice();
},
Error);
assert.throws(
() => {
testHandleScope.NewScopeWithException(() => { throw new RangeError(); });
@@ -29,6 +29,19 @@ napi_value NewScopeEscape(napi_env env, napi_callback_info info) {
return escapee;
}
napi_value NewScopeEscapeTwice(napi_env env, napi_callback_info info) {
napi_escapable_handle_scope scope;
napi_value output = NULL;
napi_value escapee = NULL;
NAPI_CALL(env, napi_open_escapable_handle_scope(env, &scope));
NAPI_CALL(env, napi_create_object(env, &output));
NAPI_CALL(env, napi_escape_handle(env, scope, output, &escapee));
NAPI_CALL(env, napi_escape_handle(env, scope, output, &escapee));
NAPI_CALL(env, napi_close_escapable_handle_scope(env, scope));
return escapee;
}
napi_value NewScopeWithException(napi_env env, napi_callback_info info) {
napi_handle_scope scope;
size_t argc;
@@ -57,6 +70,7 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
napi_property_descriptor properties[] = {
DECLARE_NAPI_PROPERTY("NewScope", NewScope),
DECLARE_NAPI_PROPERTY("NewScopeEscape", NewScopeEscape),
DECLARE_NAPI_PROPERTY("NewScopeEscapeTwice", NewScopeEscapeTwice),
DECLARE_NAPI_PROPERTY("NewScopeWithException", NewScopeWithException),
};

0 comments on commit 3e18c49

Please sign in to comment.