n-api: check against invalid handle scope usage

Fixes: nodejs/node#16175 PR-URL: nodejs/node#16201 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
ayojs · Oct 18, 2017 · fd8ca8f · fd8ca8f
1 parent d669de5
commit fd8ca8f
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 8 deletions.
diff --git a/src/node_api.cc b/src/node_api.cc
@@ -42,6 +42,7 @@ struct napi_env__ {
   v8::Persistent<v8::ObjectTemplate> function_data_template;
   v8::Persistent<v8::ObjectTemplate> accessor_data_template;
   napi_extended_error_info last_error;
+  int open_handle_scopes = 0;
 };
 
 #define ENV_OBJECT_TEMPLATE(env, prefix, destination, field_count) \
@@ -499,12 +500,16 @@ class CallbackWrapperBase : public CallbackWrapper {
     // Make sure any errors encountered last time we were in N-API are gone.
     napi_clear_last_error(env);
 
+    int open_handle_scopes = env->open_handle_scopes;
+
     napi_value result = cb(env, cbinfo_wrapper);
 
     if (result != nullptr) {
       this->SetReturnValue(result);
     }
 
+    CHECK_EQ(env->open_handle_scopes, open_handle_scopes);
+
     if (!env->last_exception.IsEmpty()) {
       isolate->ThrowException(
           v8::Local<v8::Value>::New(isolate, env->last_exception));
@@ -2602,6 +2607,7 @@ napi_status napi_open_handle_scope(napi_env env, napi_handle_scope* result) {
 
   *result = v8impl::JsHandleScopeFromV8HandleScope(
       new v8impl::HandleScopeWrapper(env->isolate));
+  env->open_handle_scopes++;
   return napi_clear_last_error(env);
 }
 
@@ -2610,7 +2616,11 @@ napi_status napi_close_handle_scope(napi_env env, napi_handle_scope scope) {
   // JS exceptions.
   CHECK_ENV(env);
   CHECK_ARG(env, scope);
+  if (env->open_handle_scopes == 0) {
+    return napi_handle_scope_mismatch;
+  }
 
+  env->open_handle_scopes--;
   delete v8impl::V8HandleScopeFromJsHandleScope(scope);
   return napi_clear_last_error(env);
 }
@@ -2625,6 +2635,7 @@ napi_status napi_open_escapable_handle_scope(
 
   *result = v8impl::JsEscapableHandleScopeFromV8EscapableHandleScope(
       new v8impl::EscapableHandleScopeWrapper(env->isolate));
+  env->open_handle_scopes++;
   return napi_clear_last_error(env);
 }
 
@@ -2635,8 +2646,12 @@ napi_status napi_close_escapable_handle_scope(
   // JS exceptions.
   CHECK_ENV(env);
   CHECK_ARG(env, scope);
+  if (env->open_handle_scopes == 0) {
+    return napi_handle_scope_mismatch;
+  }
 
   delete v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope);
+  env->open_handle_scopes--;
   return napi_clear_last_error(env);
 }
 

diff --git a/src/node_api_types.h b/src/node_api_types.h
@@ -69,7 +69,8 @@ typedef enum {
   napi_generic_failure,
   napi_pending_exception,
   napi_cancelled,
-  napi_escape_called_twice
+  napi_escape_called_twice,
+  napi_handle_scope_mismatch
 } napi_status;
 
 typedef napi_value (*napi_callback)(napi_env env,

diff --git a/test/addons-napi/test_handle_scope/test.js b/test/addons-napi/test_handle_scope/test.js
@@ -10,11 +10,7 @@ testHandleScope.NewScope();
 
 assert.ok(testHandleScope.NewScopeEscape() instanceof Object);
 
-assert.throws(
-  () => {
-    testHandleScope.NewScopeEscapeTwice();
-  },
-  Error);
+testHandleScope.NewScopeEscapeTwice();
 
 assert.throws(
   () => {

diff --git a/test/addons-napi/test_handle_scope/test_handle_scope.c b/test/addons-napi/test_handle_scope/test_handle_scope.c
@@ -33,13 +33,15 @@ napi_value NewScopeEscapeTwice(napi_env env, napi_callback_info info) {
   napi_escapable_handle_scope scope;
   napi_value output = NULL;
   napi_value escapee = NULL;
+  napi_status status;
 
   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));
+  status = napi_escape_handle(env, scope, output, &escapee);
+  NAPI_ASSERT(env, status == napi_escape_called_twice, "Escaping twice fails");
   NAPI_CALL(env, napi_close_escapable_handle_scope(env, scope));
-  return escapee;
+  return NULL;
 }
 
 napi_value NewScopeWithException(napi_env env, napi_callback_info info) {