Merge pull request #499 from gaudryc/security/fix_remote_endpoint_exception_in_webserver

Fix a security issue (DOS) in webserver

Author: gizmocuz

History.txt

@@ -79,6 +79,7 @@ Version 2.4xxx
 - Fixed: Fix Security devices edit form by hiding Selector switch options (#473).
 - Fixed: Fix missing Selector switch levels in MQTT device info response by returning all device options (#459)
 - Fixed: Fix dead code in the AsyncSerial::close method causing the method not to stop or close the background resources.
+- Fixed: Fix a security issue (DOS) in webserver to prevent a remote_endpoint exception to be thrown to the WebServer class causing the webserver to be down (no possible stop and no possible connection). Now, the webserver can be scanned using the nmap tool (for the SSL configuration purpose).
 - Updated: OpenZWave, configuration files
 
 Version 2.3530 (November 1th 2015)

webserver/connection.cpp

@@ -75,20 +75,30 @@ boost::asio::ip::tcp::socket& connection::socket()
 
 void connection::start()
 {
-    host_endpoint_ = socket().remote_endpoint().address().to_string();
-    if (secure_) {
+	boost::system::error_code ec;
+	boost::asio::ip::tcp::endpoint endpoint = socket().remote_endpoint(ec);
+	if (ec) {
+		// Prevent the exception to be thrown to run to avoid the server to be locked (still listening but no more connection or stop).
+		// If the exception returns to WebServer to also create a exception loop.
+		_log.Log(LOG_ERROR,"Getting error '%s' while getting remote_endpoint in connection::start", ec.message().c_str());
+		connection_manager_.stop(shared_from_this());
+		return;
+	}
+
+	host_endpoint_ = endpoint.address().to_string();
+	if (secure_) {
 #ifdef NS_ENABLE_SSL
 		// with ssl, we first need to complete the handshake before reading
-  		sslsocket_->async_handshake(boost::asio::ssl::stream_base::server,
-        	boost::bind(&connection::handle_handshake, shared_from_this(),
-          	boost::asio::placeholders::error));
+		sslsocket_->async_handshake(boost::asio::ssl::stream_base::server,
+			boost::bind(&connection::handle_handshake, shared_from_this(),
+			boost::asio::placeholders::error));
 #endif
-    }
-    else {
+	}
+	else {
 		// start reading data
 		read_more();
-    }
-    m_lastresponse=mytime(NULL);
+	}
+	m_lastresponse=mytime(NULL);
 }
 
 void connection::stop()

webserver/connection_manager.cpp

@@ -20,21 +20,30 @@ namespace server {
 
 void connection_manager::start(connection_ptr c)
 {
-  connections_.insert(c);
-  std::string s = c->socket().remote_endpoint().address().to_string();
+	connections_.insert(c);
 
-  if (s.substr(0, 7) == "::ffff:") {
-	  s = s.substr(7);
-  }
+	boost::system::error_code ec;
+	boost::asio::ip::tcp::endpoint endpoint = c->socket().remote_endpoint(ec);
+	if (ec) {
+		// Prevent the exception to be thrown to run to avoid the server to be locked (still listening but no more connection or stop).
+		// If the exception returns to WebServer to also create a exception loop.
+		_log.Log(LOG_ERROR,"Getting error '%s' while getting remote_endpoint in connection_manager::start", ec.message().c_str());
+		stop(c);
+		return;
+	}
 
-  if (connectedips_.find(s)==connectedips_.end())
-  {
-	  //ok, this could get a very long list when running for years
-	  connectedips_.insert(s);
-	  _log.Log(LOG_STATUS,"Incoming connection from: %s", s.c_str());
-  }
+	std::string s = endpoint.address().to_string();
+	if (s.substr(0, 7) == "::ffff:") {
+		s = s.substr(7);
+	}
+	if (connectedips_.find(s) == connectedips_.end())
+	{
+		//ok, this could get a very long list when running for years
+		connectedips_.insert(s);
+		_log.Log(LOG_STATUS,"Incoming connection from: %s", s.c_str());
+	}
 
-  c->start();
+	c->start();
 }
 
 void connection_manager::stop(connection_ptr c)