Skip to content

Commit b89b97d

Browse files
bnoordhuisbnoordhuis
committed
src: fix multi-base class ObjectWrap::Unwrap<T>()
Fix pointer unwrapping when T is a class with more than one base class. Before this commit, the wrapped void* pointer was cast directly to T* without going through ObjectWrap* first, possibly leading to a class instance pointer that points to the wrong vtable. This change required some cleanup in various files; some classes used private rather than public inheritance, others didn't derive from ObjectWrap at all... Fixes nodejs#6188.
1 parent 756b622 commit b89b97d

File tree

5 files changed

+44
-29
lines changed

5 files changed

+44
-29
lines changed

‎src/node_contextify.cc‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ class ContextifyContext {
212212
const PropertyCallbackInfo<Value>& args) {
213213
HandleScope scope(node_isolate);
214214

215-
Local<Object> data = args.Data()->ToObject();
216-
ContextifyContext* ctx = ObjectWrap::Unwrap<ContextifyContext>(data);
215+
ContextifyContext* ctx = NULL;
216+
NODE_UNWRAP(args.Data().As<Object>(), ContextifyContext, ctx);
217217

218218
Local<Object> sandbox = PersistentToLocal(node_isolate, ctx->sandbox_);
219219
Local<Value> rv = sandbox->GetRealNamedProperty(property);
@@ -236,8 +236,8 @@ class ContextifyContext {
236236
const PropertyCallbackInfo<Value>& args) {
237237
HandleScope scope(node_isolate);
238238

239-
Local<Object> data = args.Data()->ToObject();
240-
ContextifyContext* ctx = ObjectWrap::Unwrap<ContextifyContext>(data);
239+
ContextifyContext* ctx = NULL;
240+
NODE_UNWRAP(args.Data().As<Object>(), ContextifyContext, ctx);
241241

242242
PersistentToLocal(node_isolate, ctx->sandbox_)->Set(property, value);
243243
}
@@ -248,8 +248,8 @@ class ContextifyContext {
248248
const PropertyCallbackInfo<Integer>& args) {
249249
HandleScope scope(node_isolate);
250250

251-
Local<Object> data = args.Data()->ToObject();
252-
ContextifyContext* ctx = ObjectWrap::Unwrap<ContextifyContext>(data);
251+
ContextifyContext* ctx = NULL;
252+
NODE_UNWRAP(args.Data().As<Object>(), ContextifyContext, ctx);
253253

254254
Local<Object> sandbox = PersistentToLocal(node_isolate, ctx->sandbox_);
255255
Local<Object> proxy_global = PersistentToLocal(node_isolate,
@@ -269,8 +269,8 @@ class ContextifyContext {
269269
const PropertyCallbackInfo<Boolean>& args) {
270270
HandleScope scope(node_isolate);
271271

272-
Local<Object> data = args.Data()->ToObject();
273-
ContextifyContext* ctx = ObjectWrap::Unwrap<ContextifyContext>(data);
272+
ContextifyContext* ctx = NULL;
273+
NODE_UNWRAP(args.Data().As<Object>(), ContextifyContext, ctx);
274274

275275
bool success = PersistentToLocal(node_isolate,
276276
ctx->sandbox_)->Delete(property);
@@ -286,15 +286,15 @@ class ContextifyContext {
286286
const PropertyCallbackInfo<Array>& args) {
287287
HandleScope scope(node_isolate);
288288

289-
Local<Object> data = args.Data()->ToObject();
290-
ContextifyContext* ctx = ObjectWrap::Unwrap<ContextifyContext>(data);
289+
ContextifyContext* ctx = NULL;
290+
NODE_UNWRAP(args.Data().As<Object>(), ContextifyContext, ctx);
291291

292292
Local<Object> sandbox = PersistentToLocal(node_isolate, ctx->sandbox_);
293293
args.GetReturnValue().Set(sandbox->GetPropertyNames());
294294
}
295295
};
296296

297-
class ContextifyScript : ObjectWrap {
297+
class ContextifyScript : public ObjectWrap {
298298
private:
299299
Persistent<Script> script_;
300300

‎src/node_crypto.cc‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,8 @@ void SSLWrap<Base>::GetPeerCertificate(
886886
const FunctionCallbackInfo<Value>& args) {
887887
HandleScope scope(node_isolate);
888888

889-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
889+
Base* w = NULL;
890+
NODE_UNWRAP(args.This(), Base, w);
890891
Environment* env = w->env();
891892

892893
Local<Object> info = Object::New();
@@ -1020,7 +1021,8 @@ template <class Base>
10201021
void SSLWrap<Base>::GetSession(const FunctionCallbackInfo<Value>& args) {
10211022
HandleScope scope(node_isolate);
10221023

1023-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1024+
Base* w = NULL;
1025+
NODE_UNWRAP(args.This(), Base, w);
10241026

10251027
SSL_SESSION* sess = SSL_get_session(w->ssl_);
10261028
if (sess == NULL)
@@ -1041,7 +1043,8 @@ template <class Base>
10411043
void SSLWrap<Base>::SetSession(const FunctionCallbackInfo<Value>& args) {
10421044
HandleScope scope(node_isolate);
10431045

1044-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1046+
Base* w = NULL;
1047+
NODE_UNWRAP(args.This(), Base, w);
10451048

10461049
if (args.Length() < 1 ||
10471050
(!args[0]->IsString() && !Buffer::HasInstance(args[0]))) {
@@ -1079,7 +1082,8 @@ template <class Base>
10791082
void SSLWrap<Base>::LoadSession(const FunctionCallbackInfo<Value>& args) {
10801083
HandleScope scope(node_isolate);
10811084

1082-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1085+
Base* w = NULL;
1086+
NODE_UNWRAP(args.This(), Base, w);
10831087
Environment* env = w->env();
10841088

10851089
if (args.Length() >= 1 && Buffer::HasInstance(args[0])) {
@@ -1111,7 +1115,8 @@ void SSLWrap<Base>::LoadSession(const FunctionCallbackInfo<Value>& args) {
11111115
template <class Base>
11121116
void SSLWrap<Base>::IsSessionReused(const FunctionCallbackInfo<Value>& args) {
11131117
HandleScope scope(node_isolate);
1114-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1118+
Base* w = NULL;
1119+
NODE_UNWRAP(args.This(), Base, w);
11151120
bool yes = SSL_session_reused(w->ssl_);
11161121
args.GetReturnValue().Set(yes);
11171122
}
@@ -1120,7 +1125,8 @@ void SSLWrap<Base>::IsSessionReused(const FunctionCallbackInfo<Value>& args) {
11201125
template <class Base>
11211126
void SSLWrap<Base>::ReceivedShutdown(const FunctionCallbackInfo<Value>& args) {
11221127
HandleScope scope(node_isolate);
1123-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1128+
Base* w = NULL;
1129+
NODE_UNWRAP(args.This(), Base, w);
11241130
bool yes = SSL_get_shutdown(w->ssl_) == SSL_RECEIVED_SHUTDOWN;
11251131
args.GetReturnValue().Set(yes);
11261132
}
@@ -1129,9 +1135,8 @@ void SSLWrap<Base>::ReceivedShutdown(const FunctionCallbackInfo<Value>& args) {
11291135
template <class Base>
11301136
void SSLWrap<Base>::EndParser(const FunctionCallbackInfo<Value>& args) {
11311137
HandleScope scope(node_isolate);
1132-
1133-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1134-
1138+
Base* w = NULL;
1139+
NODE_UNWRAP(args.This(), Base, w);
11351140
w->hello_parser_.End();
11361141
}
11371142

@@ -1140,7 +1145,8 @@ template <class Base>
11401145
void SSLWrap<Base>::Renegotiate(const FunctionCallbackInfo<Value>& args) {
11411146
HandleScope scope(node_isolate);
11421147

1143-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1148+
Base* w = NULL;
1149+
NODE_UNWRAP(args.This(), Base, w);
11441150

11451151
ClearErrorOnReturn clear_error_on_return;
11461152
(void) &clear_error_on_return; // Silence unused variable warning.
@@ -1153,7 +1159,8 @@ void SSLWrap<Base>::Renegotiate(const FunctionCallbackInfo<Value>& args) {
11531159
template <class Base>
11541160
void SSLWrap<Base>::IsInitFinished(const FunctionCallbackInfo<Value>& args) {
11551161
HandleScope scope(node_isolate);
1156-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1162+
Base* w = NULL;
1163+
NODE_UNWRAP(args.This(), Base, w);
11571164
bool yes = SSL_is_init_finished(w->ssl_);
11581165
args.GetReturnValue().Set(yes);
11591166
}
@@ -1164,7 +1171,8 @@ template <class Base>
11641171
void SSLWrap<Base>::VerifyError(const FunctionCallbackInfo<Value>& args) {
11651172
HandleScope scope(node_isolate);
11661173

1167-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1174+
Base* w = NULL;
1175+
NODE_UNWRAP(args.This(), Base, w);
11681176

11691177
// XXX(indutny) Do this check in JS land?
11701178
X509* peer_cert = SSL_get_peer_certificate(w->ssl_);
@@ -1230,7 +1238,8 @@ template <class Base>
12301238
void SSLWrap<Base>::GetCurrentCipher(const FunctionCallbackInfo<Value>& args) {
12311239
HandleScope scope(node_isolate);
12321240

1233-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1241+
Base* w = NULL;
1242+
NODE_UNWRAP(args.This(), Base, w);
12341243
Environment* env = w->env();
12351244

12361245
OPENSSL_CONST SSL_CIPHER* c = SSL_get_current_cipher(w->ssl_);
@@ -1325,7 +1334,8 @@ void SSLWrap<Base>::GetNegotiatedProto(
13251334
const FunctionCallbackInfo<Value>& args) {
13261335
HandleScope scope(node_isolate);
13271336

1328-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1337+
Base* w = NULL;
1338+
NODE_UNWRAP(args.This(), Base, w);
13291339

13301340
if (w->is_client()) {
13311341
if (w->selected_npn_proto_.IsEmpty() == false) {
@@ -1351,7 +1361,8 @@ template <class Base>
13511361
void SSLWrap<Base>::SetNPNProtocols(const FunctionCallbackInfo<Value>& args) {
13521362
HandleScope scope(node_isolate);
13531363

1354-
Base* w = ObjectWrap::Unwrap<Base>(args.This());
1364+
Base* w = NULL;
1365+
NODE_UNWRAP(args.This(), Base, w);
13551366

13561367
if (args.Length() < 1 || !Buffer::HasInstance(args[0]))
13571368
return ThrowTypeError("Must give a Buffer as first argument");

‎src/node_crypto.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ extern X509_STORE* root_cert_store;
5757
// Forward declaration
5858
class Connection;
5959

60-
class SecureContext : ObjectWrap {
60+
class SecureContext : public ObjectWrap {
6161
public:
6262
static void Initialize(Environment* env, v8::Handle<v8::Object> target);
6363

‎src/node_object_wrap.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ class NODE_EXTERN ObjectWrap {
5656
static inline T* Unwrap(v8::Handle<v8::Object> handle) {
5757
assert(!handle.IsEmpty());
5858
assert(handle->InternalFieldCount() > 0);
59-
return static_cast<T*>(handle->GetAlignedPointerFromInternalField(0));
59+
// Cast to ObjectWrap before casting to T. A direct cast from void
60+
// to T won't work right when T has more than one base class.
61+
void* ptr = handle->GetAlignedPointerFromInternalField(0);
62+
ObjectWrap* wrap = static_cast<ObjectWrap*>(ptr);
63+
return static_cast<T*>(wrap);
6064
}
6165

6266

‎src/node_stat_watcher.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030

3131
namespace node {
3232

33-
class StatWatcher : ObjectWrap {
33+
class StatWatcher : public ObjectWrap {
3434
public:
3535
static void Initialize(v8::Handle<v8::Object> target);
3636
inline Environment* env() const { return env_; }

0 commit comments

Comments
 (0)