tls_wrap: fix use after free

Do not free TLSCallbacks from StreamWrap. TLSCallbacks is bound to a V8 object and should be collected by V8's GC.
nodejs · Sep 3, 2014 · 7343c77 · 7343c77
1 parent 68c14d6
commit 7343c77
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 5 deletions.
diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
@@ -63,7 +63,8 @@ StreamWrap::StreamWrap(Environment* env,
     : HandleWrap(env, object, reinterpret_cast<uv_handle_t*>(stream), provider),
       stream_(stream),
       default_callbacks_(this),
-      callbacks_(&default_callbacks_) {
+      callbacks_(&default_callbacks_),
+      callbacks_gc_(false) {
 }
 
 

diff --git a/src/stream_wrap.h b/src/stream_wrap.h
@@ -105,9 +105,10 @@ class StreamWrapCallbacks {
 
 class StreamWrap : public HandleWrap {
  public:
-  void OverrideCallbacks(StreamWrapCallbacks* callbacks) {
+  void OverrideCallbacks(StreamWrapCallbacks* callbacks, bool gc) {
     StreamWrapCallbacks* old = callbacks_;
     callbacks_ = callbacks;
+    callbacks_gc_ = gc;
     if (old != &default_callbacks_)
       delete old;
   }
@@ -160,10 +161,10 @@ class StreamWrap : public HandleWrap {
              AsyncWrap::ProviderType provider);
 
   ~StreamWrap() {
-    if (callbacks_ != &default_callbacks_) {
+    if (!callbacks_gc_ && callbacks_ != &default_callbacks_) {
       delete callbacks_;
-      callbacks_ = NULL;
     }
+    callbacks_ = NULL;
   }
 
   void StateChange() { }
@@ -191,6 +192,7 @@ class StreamWrap : public HandleWrap {
   uv_stream_t* const stream_;
   StreamWrapCallbacks default_callbacks_;
   StreamWrapCallbacks* callbacks_;  // Overridable callbacks
+  bool callbacks_gc_;
 
   friend class StreamWrapCallbacks;
 };

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -225,7 +225,7 @@ void TLSCallbacks::Wrap(const FunctionCallbackInfo<Value>& args) {
   TLSCallbacks* callbacks = NULL;
   WITH_GENERIC_STREAM(env, stream, {
     callbacks = new TLSCallbacks(env, kind, sc, wrap->callbacks());
-    wrap->OverrideCallbacks(callbacks);
+    wrap->OverrideCallbacks(callbacks, true);
   });
 
   if (callbacks == NULL) {