From d89ff1b63d6792a25af872746013387001ebb72b Mon Sep 17 00:00:00 2001 From: Miguel Portilla Date: Thu, 5 Jul 2018 20:35:57 -0400 Subject: [PATCH] Handle websocket construction exceptions: Certain versions of the Beast HTTP & WebSocket library can generate exceptions, which unless caught, will result in unexpected behavior. Acknowledgements: Ripple thanks Thomas Snider for originally noticing this issue and responsibly disclosing it to Ripple. Bug Bounties and Responsible Disclosures: We welcome reviews of the rippled code and urge researchers to responsibly disclose any issues that they may find. For more on Ripple's Bug Bounty program, please visit: https://ripple.com/bug-bounty --- src/ripple/rpc/impl/ServerHandlerImp.cpp | 106 +++++++++-------------- src/ripple/rpc/impl/ServerHandlerImp.h | 24 +++-- src/ripple/server/impl/PlainHTTPPeer.h | 2 +- src/test/server/Server_test.cpp | 10 +-- 4 files changed, 62 insertions(+), 80 deletions(-) diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 52ec11bd76..967efcd160 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -65,14 +65,14 @@ isStatusRequest( static Handoff -unauthorizedResponse( - http_request_type const& request) +statusRequestResponse( + http_request_type const& request, boost::beast::http::status status) { using namespace boost::beast::http; Handoff handoff; response msg; msg.version(request.version()); - msg.result(boost::beast::http::status::unauthorized); + msg.result(status); msg.insert("Server", BuildInfo::getFullVersionString()); msg.insert("Content-Type", "text/html"); msg.insert("Connection", "close"); @@ -167,82 +167,58 @@ ServerHandlerImp::onAccept (Session& session, return true; } -auto -ServerHandlerImp::onHandoff (Session& session, - std::unique_ptr && bundle, - http_request_type&& request, - boost::asio::ip::tcp::endpoint remote_address) -> - Handoff +Handoff +ServerHandlerImp::onHandoff( + Session& session, + std::unique_ptr&& bundle, + http_request_type&& request, + boost::asio::ip::tcp::endpoint const& remote_address) { - const bool is_ws = - (session.port().protocol.count("wss") > 0) || - (session.port().protocol.count("wss2") > 0); + using namespace boost::beast; + auto const& p {session.port().protocol}; + bool const is_ws { + p.count("ws") > 0 || p.count("ws2") > 0 || + p.count("wss") > 0 || p.count("wss2") > 0}; - if(boost::beast::websocket::is_upgrade(request)) + if(websocket::is_upgrade(request)) { - if(is_ws) + if (!is_ws) + return statusRequestResponse(request, http::status::unauthorized); + + std::shared_ptr ws; + try { - Handoff handoff; - auto const ws = session.websocketUpgrade(); - auto is = std::make_shared(m_networkOPs, ws); - is->getConsumer() = requestInboundEndpoint( - m_resourceManager, - beast::IPAddressConversion::from_asio(remote_address), - session.port(), is->user()); - ws->appDefined = std::move(is); - ws->run(); - handoff.moved = true; - return handoff; + ws = session.websocketUpgrade(); + } + catch (std::exception const& e) + { + JLOG(m_journal.error()) + << "Exception upgrading websocket: " << e.what() << "\n"; + return statusRequestResponse(request, + http::status::internal_server_error); } - return unauthorizedResponse(request); + auto is {std::make_shared(m_networkOPs, ws)}; + is->getConsumer() = requestInboundEndpoint( + m_resourceManager, + beast::IPAddressConversion::from_asio(remote_address), + session.port(), + is->user()); + ws->appDefined = std::move(is); + ws->run(); + + Handoff handoff; + handoff.moved = true; + return handoff; } - if(session.port().protocol.count("peer") > 0) - { + if (bundle && p.count("peer") > 0) return app_.overlay().onHandoff(std::move(bundle), std::move(request), remote_address); - } if (is_ws && isStatusRequest(request)) return statusResponse(request); - // Pass to legacy onRequest - return {}; -} - -auto -ServerHandlerImp::onHandoff (Session& session, - boost::asio::ip::tcp::socket&& socket, - http_request_type&& request, - boost::asio::ip::tcp::endpoint remote_address) -> - Handoff -{ - if(boost::beast::websocket::is_upgrade(request)) - { - if (session.port().protocol.count("ws2") > 0 || - session.port().protocol.count("ws") > 0) - { - Handoff handoff; - auto const ws = session.websocketUpgrade(); - auto is = std::make_shared(m_networkOPs, ws); - is->getConsumer() = requestInboundEndpoint( - m_resourceManager, beast::IPAddressConversion::from_asio( - remote_address), session.port(), is->user()); - ws->appDefined = std::move(is); - ws->run(); - handoff.moved = true; - return handoff; - } - - return unauthorizedResponse(request); - } - - if ((session.port().protocol.count("ws") > 0 || - session.port().protocol.count("ws2") > 0) && - isStatusRequest(request)) - return statusResponse(request); - // Otherwise pass to legacy onRequest or websocket return {}; } diff --git a/src/ripple/rpc/impl/ServerHandlerImp.h b/src/ripple/rpc/impl/ServerHandlerImp.h index 1441d0cf33..192eaeaf8e 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.h +++ b/src/ripple/rpc/impl/ServerHandlerImp.h @@ -132,15 +132,25 @@ public: boost::asio::ip::tcp::endpoint endpoint); Handoff - onHandoff (Session& session, - std::unique_ptr && bundle, - http_request_type&& request, - boost::asio::ip::tcp::endpoint remote_address); + onHandoff( + Session& session, + std::unique_ptr&& bundle, + http_request_type&& request, + boost::asio::ip::tcp::endpoint const& remote_address); Handoff - onHandoff (Session& session, boost::asio::ip::tcp::socket&& socket, + onHandoff( + Session& session, http_request_type&& request, - boost::asio::ip::tcp::endpoint remote_address); + boost::asio::ip::tcp::endpoint const& remote_address) + { + return onHandoff( + session, + {}, + std::forward(request), + remote_address); + } + void onRequest (Session& session); @@ -174,8 +184,6 @@ private: Handoff statusResponse(http_request_type const& request) const; - - }; } diff --git a/src/ripple/server/impl/PlainHTTPPeer.h b/src/ripple/server/impl/PlainHTTPPeer.h index 735511398b..5c2984c0e6 100644 --- a/src/ripple/server/impl/PlainHTTPPeer.h +++ b/src/ripple/server/impl/PlainHTTPPeer.h @@ -118,7 +118,7 @@ do_request() { ++this->request_count_; auto const what = this->handler_.onHandoff(this->session(), - std::move(stream_), std::move(this->message_), this->remote_address_); + std::move(this->message_), this->remote_address_); if (what.moved) return; boost::system::error_code ec; diff --git a/src/test/server/Server_test.cpp b/src/test/server/Server_test.cpp index cdb79ff7e0..8acaf45a5c 100644 --- a/src/test/server/Server_test.cpp +++ b/src/test/server/Server_test.cpp @@ -111,9 +111,8 @@ public: } Handoff - onHandoff (Session& session, boost::asio::ip::tcp::socket&& socket, - http_request_type&& request, - boost::asio::ip::tcp::endpoint remote_address) + onHandoff (Session& session, http_request_type&& request, + boost::asio::ip::tcp::endpoint remote_address) { return Handoff{}; } @@ -317,9 +316,8 @@ public: } Handoff - onHandoff (Session& session, boost::asio::ip::tcp::socket&& socket, - http_request_type&& request, - boost::asio::ip::tcp::endpoint remote_address) + onHandoff (Session& session, http_request_type&& request, + boost::asio::ip::tcp::endpoint remote_address) { return Handoff{}; }