Check value is external in napi_unwrap (#210)

- Check that an internal field value is actually an External before casting. This prevents a crash if the wrong object is passed to napi_unwrap. - Wrap some macro parameters in parentheses.
nodejs · Mar 29, 2017 · c9f1143 · c9f1143
1 parent f4bbfee
commit c9f1143
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 3 deletions.
diff --git a/src/node_api.cc b/src/node_api.cc
@@ -1857,7 +1857,10 @@ napi_status napi_unwrap(napi_env env, napi_value js_object, void** result) {
   // via napi_define_class() can be (un)wrapped.
   RETURN_STATUS_IF_FALSE(obj->InternalFieldCount() > 0, napi_invalid_arg);
 
-  *result = v8::Local<v8::External>::Cast(obj->GetInternalField(0))->Value();
+  v8::Local<v8::Value> unwrappedValue = obj->GetInternalField(0);
+  RETURN_STATUS_IF_FALSE(unwrappedValue->IsExternal(), napi_invalid_arg);
+
+  *result = unwrappedValue.As<v8::External>()->Value();
 
   return napi_ok;
 }

diff --git a/test/addons-napi/3_callbacks/binding.c b/test/addons-napi/3_callbacks/binding.c
@@ -1,7 +1,7 @@
 #include <node_api.h>
 
 #define NAPI_CALL(theCall)                                                \
-  if (theCall != napi_ok) {                                               \
+  if ((theCall) != napi_ok) {                                             \
     const char* errorMessage = napi_get_last_error_info()->error_message; \
     errorMessage = errorMessage ? errorMessage : "empty error message";   \
     napi_throw_error((env), errorMessage);                                \

diff --git a/test/addons-napi/test_buffer/test_buffer.c b/test/addons-napi/test_buffer/test_buffer.c
@@ -11,7 +11,7 @@
   }
 
 #define NAPI_CALL(env, theCall)                                           \
-  if (theCall != napi_ok) {                                               \
+  if ((theCall) != napi_ok) {                                             \
     const char* errorMessage = napi_get_last_error_info()->error_message; \
     errorMessage = errorMessage ? errorMessage : "empty error message";   \
     napi_throw_error((env), errorMessage);                                \