From 3e18c49657bcb0f2504def3d7d6d900f0c25e357 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 13 Jun 2017 00:40:00 -0400 Subject: 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: https://github.com/nodejs/node/pull/13651 Reviewed-By: Jason Ginchereau Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis --- src/node_api.cc | 29 ++++++++++++++++++++++------- src/node_api_types.h | 2 +- 2 files changed, 23 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/node_api.cc b/src/node_api.cc index 8978997480..18ff7b64e4 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -17,6 +17,7 @@ #include #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 v8::Local Escape(v8::Local 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, diff --git a/src/node_api_types.h b/src/node_api_types.h index 4bf1b82631..43102c519c 100644 --- a/src/node_api_types.h +++ b/src/node_api_types.h @@ -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, -- cgit v1.2.3