From 27609327eecffa1541183221cde8c398e2ce316c Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Wed, 19 Nov 2014 01:59:09 +0900
Subject: [PATCH] nghttpx: Fix heap-after-free crash in https upstream

Add Upstream::on_handler_delete() hook to safely write log for
HttpsUpstream.
---
 src/shrpx_client_handler.cc |  4 ++++
 src/shrpx_http2_upstream.cc |  3 +++
 src/shrpx_http2_upstream.h  |  2 ++
 src/shrpx_https_upstream.cc | 15 +++++++++------
 src/shrpx_https_upstream.h  |  2 ++
 src/shrpx_spdy_upstream.cc  |  3 +++
 src/shrpx_spdy_upstream.h   |  2 ++
 src/shrpx_upstream.h        |  2 ++
 8 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc
index 732d5f84..c66033d8 100644
--- a/src/shrpx_client_handler.cc
+++ b/src/shrpx_client_handler.cc
@@ -221,6 +221,10 @@ ClientHandler::~ClientHandler()
     CLOG(INFO, this) << "Deleting";
   }
 
+  if(upstream_) {
+    upstream_->on_handler_delete();
+  }
+
   --worker_stat_->num_connections;
 
   // TODO If backend is http/2, and it is in CONNECTED state, signal
diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc
index 54f12b83..d6a2884f 100644
--- a/src/shrpx_http2_upstream.cc
+++ b/src/shrpx_http2_upstream.cc
@@ -1466,4 +1466,7 @@ void Http2Upstream::reset_timeouts()
                                   &get_config()->upstream_write_timeout);
 }
 
+void Http2Upstream::on_handler_delete()
+{}
+
 } // namespace shrpx
diff --git a/src/shrpx_http2_upstream.h b/src/shrpx_http2_upstream.h
index 0b846cf3..59313eb9 100644
--- a/src/shrpx_http2_upstream.h
+++ b/src/shrpx_http2_upstream.h
@@ -74,6 +74,8 @@ public:
                                  const uint8_t *data, size_t len, bool flush);
   virtual int on_downstream_body_complete(Downstream *downstream);
 
+  virtual void on_handler_delete();
+
   virtual void reset_timeouts();
 
   bool get_flow_control() const;
diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc
index e4a2d5a4..a4ad2209 100644
--- a/src/shrpx_https_upstream.cc
+++ b/src/shrpx_https_upstream.cc
@@ -57,12 +57,7 @@ HttpsUpstream::HttpsUpstream(ClientHandler *handler)
 }
 
 HttpsUpstream::~HttpsUpstream()
-{
-  if(downstream_ &&
-     downstream_->get_response_state() == Downstream::MSG_COMPLETE) {
-    handler_->write_accesslog(downstream_.get());
-  }
-}
+{}
 
 void HttpsUpstream::reset_current_header_length()
 {
@@ -977,4 +972,12 @@ void HttpsUpstream::reset_timeouts()
                                   &get_config()->upstream_write_timeout);
 }
 
+void HttpsUpstream::on_handler_delete()
+{
+  if(downstream_ &&
+     downstream_->get_response_state() == Downstream::MSG_COMPLETE) {
+    handler_->write_accesslog(downstream_.get());
+  }
+}
+
 } // namespace shrpx
diff --git a/src/shrpx_https_upstream.h b/src/shrpx_https_upstream.h
index 226c0305..8e151041 100644
--- a/src/shrpx_https_upstream.h
+++ b/src/shrpx_https_upstream.h
@@ -67,6 +67,8 @@ public:
                                  const uint8_t *data, size_t len, bool flush);
   virtual int on_downstream_body_complete(Downstream *downstream);
 
+  virtual void on_handler_delete();
+
   virtual void reset_timeouts();
 
   void reset_current_header_length();
diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc
index bbad5a87..dd52a5eb 100644
--- a/src/shrpx_spdy_upstream.cc
+++ b/src/shrpx_spdy_upstream.cc
@@ -1154,4 +1154,7 @@ void SpdyUpstream::reset_timeouts()
                                   &get_config()->upstream_write_timeout);
 }
 
+void SpdyUpstream::on_handler_delete()
+{}
+
 } // namespace shrpx
diff --git a/src/shrpx_spdy_upstream.h b/src/shrpx_spdy_upstream.h
index 018e69e9..84f5f845 100644
--- a/src/shrpx_spdy_upstream.h
+++ b/src/shrpx_spdy_upstream.h
@@ -73,6 +73,8 @@ public:
                                  const uint8_t *data, size_t len, bool flush);
   virtual int on_downstream_body_complete(Downstream *downstream);
 
+  virtual void on_handler_delete();
+
   virtual void reset_timeouts();
 
   bool get_flow_control() const;
diff --git a/src/shrpx_upstream.h b/src/shrpx_upstream.h
index fb623a25..2df61692 100644
--- a/src/shrpx_upstream.h
+++ b/src/shrpx_upstream.h
@@ -56,6 +56,8 @@ public:
                                  bool flush) = 0;
   virtual int on_downstream_body_complete(Downstream *downstream) = 0;
 
+  virtual void on_handler_delete() = 0;
+
   virtual void pause_read(IOCtrlReason reason) = 0;
   virtual int resume_read(IOCtrlReason reason, Downstream *downstream,
                           size_t consumed) = 0;