From 1069b4ab22eca5eea3d01992f1153a05fcf01c75 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 1 Dec 2017 12:11:41 -0800 Subject: [PATCH 1/4] Update reports for hybrid assessment --- CHANGELOG.md | 6 ++++++ doc/qbk/01_intro.qbk | 29 +++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c7a0c77..e7cf89f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +Version 144-hf1: + +* Update reports for hybrid assessment + +-------------------------------------------------------------------------------- + Version 144: * Fix websocket permessage-deflate negotiation diff --git a/doc/qbk/01_intro.qbk b/doc/qbk/01_intro.qbk index a3475705..d06643a4 100644 --- a/doc/qbk/01_intro.qbk +++ b/doc/qbk/01_intro.qbk @@ -101,7 +101,32 @@ for tirelessly answering questions on [section Reports] [block''''''] -[section WebSocket] +[section Security Review (Bishop Fox)] + +Since 2005, [@https://www.bishopfox.com/ Bishop Fox] has provided +security consulting services to the Fortune 1000, high-tech startups, +and financial institutions worldwide. +Beast engaged Bishop Fox to assess the security of the Boost C++ Beast HTTP/S +networking library. The following report details the findings identified during +the course of the engagement, which started on September 11, 2017. + +The assessment team conducted a hybrid application assessment of the Beast +library. Bishop Fox’s hybrid application assessment methodology leverages +the real-world attack techniques of application penetration testing in +combination with targeted source code review to thoroughly identify +application security vulnerabilities. These fullknowledge assessments +begin with automated scans of the deployed application and source code. +Next, analyses of the scan results are combined with manual review to +thoroughly identify potential application security vulnerabilities. In +addition, the team performs a review of the application architecture and +business logic to locate any design-level issues. Finally, the team performs +manual exploitation and review of these issues to validate the findings. + +[@https://vinniefalco.github.io/BeastAssets/Beast%20-%20Hybrid%20Application%20Assessment%202017%20-%20Assessment%20Report%20-%2020171114.pdf [*Beast - Hybrid Application Assessment 2017]] + +[endsect] + +[section WebSocket (Autobahn|Testsuite)] The [@https://github.com/crossbario/autobahn-testsuite Autobahn WebSockets Testsuite] @@ -114,7 +139,7 @@ verification and performance and limits testing. Autobahn|Testsuite is used across the industry and contains over 500 test cases. -[@https://vinniefalco.github.io/boost/beast/reports/autobahn/index.html [*Autobahn|Testsuite WebSocket Results]] +[@https://vinniefalco.github.io/BeastAssets/reports/autobahn/index.html [*Autobahn|Testsuite WebSocket Results]] [warning Version 0.7.6 of Autobahn|Testsuite contains a From 2f451badc8370f3ad125a821fd9ca1098b1b2e92 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 1 Dec 2017 14:06:21 -0800 Subject: [PATCH 2/4] Handle invalid deflate frames: This fixes a problem where an assert is generated or an error is ignored when an invalid deflate stream is produced after appending the final empty deflate block. --- CHANGELOG.md | 1 + include/boost/beast/websocket/error.hpp | 5 +- include/boost/beast/websocket/impl/error.ipp | 1 + include/boost/beast/websocket/impl/read.ipp | 32 ++-- include/boost/beast/zlib/impl/error.ipp | 2 +- test/beast/websocket/error.cpp | 1 + test/beast/websocket/read.cpp | 182 +++++++++++++++++++ 7 files changed, 206 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7cf89f0..ca53dfb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ Version 144-hf1: * Update reports for hybrid assessment +* Handle invalid deflate frames -------------------------------------------------------------------------------- diff --git a/include/boost/beast/websocket/error.hpp b/include/boost/beast/websocket/error.hpp index 4e367ff4..139a2087 100644 --- a/include/boost/beast/websocket/error.hpp +++ b/include/boost/beast/websocket/error.hpp @@ -30,7 +30,10 @@ enum class error handshake_failed, /// buffer overflow - buffer_overflow + buffer_overflow, + + /// partial deflate block + partial_deflate_block }; } // websocket diff --git a/include/boost/beast/websocket/impl/error.ipp b/include/boost/beast/websocket/impl/error.ipp index 9acd0c1d..ed18829c 100644 --- a/include/boost/beast/websocket/impl/error.ipp +++ b/include/boost/beast/websocket/impl/error.ipp @@ -43,6 +43,7 @@ public: case error::closed: return "WebSocket connection closed normally"; case error::handshake_failed: return "WebSocket upgrade handshake failed"; case error::buffer_overflow: return "WebSocket dynamic buffer overflow"; + case error::partial_deflate_block: return "WebSocket partial deflate block"; } } diff --git a/include/boost/beast/websocket/impl/read.ipp b/include/boost/beast/websocket/impl/read.ipp index b61087e6..422cc376 100644 --- a/include/boost/beast/websocket/impl/read.ipp +++ b/include/boost/beast/websocket/impl/read.ipp @@ -512,13 +512,14 @@ operator()( zs.next_in = empty_block; zs.avail_in = sizeof(empty_block); ws_.pmd_->zi.write(zs, zlib::Flush::sync, ec); - BOOST_ASSERT(! ec); - // VFALCO See: - // https://github.com/madler/zlib/issues/280 - BOOST_ASSERT(zs.total_out == 0); - cb_.consume(zs.total_out); - ws_.rd_size_ += zs.total_out; - bytes_written_ += zs.total_out; + if(! ec) + { + // https://github.com/madler/zlib/issues/280 + if(zs.total_out > 0) + ec = error::partial_deflate_block; + } + if(! ws_.check_ok(ec)) + goto upcall; if( (ws_.role_ == role_type::client && ws_.pmd_config_.server_no_context_takeover) || @@ -533,7 +534,6 @@ operator()( break; } ws_.pmd_->zi.write(zs, zlib::Flush::sync, ec); - BOOST_ASSERT(ec != zlib::error::end_of_stream); if(! ws_.check_ok(ec)) goto upcall; if(ws_.rd_msg_max_ && beast::detail::sum_exceeds( @@ -1239,13 +1239,14 @@ loop: zs.next_in = empty_block; zs.avail_in = sizeof(empty_block); pmd_->zi.write(zs, zlib::Flush::sync, ec); - BOOST_ASSERT(! ec); - // VFALCO See: - // https://github.com/madler/zlib/issues/280 - BOOST_ASSERT(zs.total_out == 0); - cb.consume(zs.total_out); - rd_size_ += zs.total_out; - bytes_written += zs.total_out; + if(! ec) + { + // https://github.com/madler/zlib/issues/280 + if(zs.total_out > 0) + ec = error::partial_deflate_block; + } + if(! check_ok(ec)) + return bytes_written; if( (role_ == role_type::client && pmd_config_.server_no_context_takeover) || @@ -1260,7 +1261,6 @@ loop: break; } pmd_->zi.write(zs, zlib::Flush::sync, ec); - BOOST_ASSERT(ec != zlib::error::end_of_stream); if(! check_ok(ec)) return bytes_written; if(rd_msg_max_ && beast::detail::sum_exceeds( diff --git a/include/boost/beast/zlib/impl/error.ipp b/include/boost/beast/zlib/impl/error.ipp index c14de15e..6aa3d22d 100644 --- a/include/boost/beast/zlib/impl/error.ipp +++ b/include/boost/beast/zlib/impl/error.ipp @@ -69,7 +69,7 @@ public: switch(static_cast(ev)) { case error::need_buffers: return "need buffers"; - case error::end_of_stream: return "end of stream"; + case error::end_of_stream: return "unexpected end of deflate stream"; case error::stream_error: return "stream error"; case error::invalid_block_type: return "invalid block type"; diff --git a/test/beast/websocket/error.cpp b/test/beast/websocket/error.cpp index bb2bea0f..c7d6614f 100644 --- a/test/beast/websocket/error.cpp +++ b/test/beast/websocket/error.cpp @@ -41,6 +41,7 @@ public: check("boost.beast.websocket", error::failed); check("boost.beast.websocket", error::handshake_failed); check("boost.beast.websocket", error::buffer_overflow); + check("boost.beast.websocket", error::partial_deflate_block); } }; diff --git a/test/beast/websocket/read.cpp b/test/beast/websocket/read.cpp index 577fb554..6ec1cbff 100644 --- a/test/beast/websocket/read.cpp +++ b/test/beast/websocket/read.cpp @@ -14,6 +14,8 @@ #include +#include + namespace boost { namespace beast { namespace websocket { @@ -1044,6 +1046,184 @@ public: BEAST_EXPECT(n == 0); } + /* Bishop Fox Hybrid Assessment issue 1 + + Happens with permessage-deflate enabled and a + compressed frame with the FIN bit set ends with an + invalid prefix. + */ + void + testIssueBF1() + { + permessage_deflate pmd; + pmd.client_enable = true; + pmd.server_enable = true; + + // read +#if 0 + { + echo_server es{log}; + boost::asio::io_context ioc; + stream ws{ioc}; + ws.set_option(pmd); + ws.next_layer().connect(es.stream()); + ws.handshake("localhost", "/"); + // invalid 1-byte deflate block in frame + boost::asio::write(ws.next_layer(), sbuf( + "\xc1\x81\x3a\xa1\x74\x3b\x49")); + } +#endif + { + boost::asio::io_context ioc; + stream wsc{ioc}; + stream wss{ioc}; + wsc.set_option(pmd); + wss.set_option(pmd); + wsc.next_layer().connect(wss.next_layer()); + wsc.async_handshake( + "localhost", "/", [](error_code){}); + wss.async_accept([](error_code){}); + ioc.run(); + ioc.restart(); + BEAST_EXPECT(wsc.is_open()); + BEAST_EXPECT(wss.is_open()); + // invalid 1-byte deflate block in frame + boost::asio::write(wsc.next_layer(), sbuf( + "\xc1\x81\x3a\xa1\x74\x3b\x49")); + error_code ec; + multi_buffer b; + wss.read(b, ec); + BEAST_EXPECTS(ec == zlib::error::end_of_stream, ec.message()); + } + + // async read +#if 0 + { + echo_server es{log, kind::async}; + boost::asio::io_context ioc; + stream ws{ioc}; + ws.set_option(pmd); + ws.next_layer().connect(es.stream()); + ws.handshake("localhost", "/"); + // invalid 1-byte deflate block in frame + boost::asio::write(ws.next_layer(), sbuf( + "\xc1\x81\x3a\xa1\x74\x3b\x49")); + } +#endif + { + boost::asio::io_context ioc; + stream wsc{ioc}; + stream wss{ioc}; + wsc.set_option(pmd); + wss.set_option(pmd); + wsc.next_layer().connect(wss.next_layer()); + wsc.async_handshake( + "localhost", "/", [](error_code){}); + wss.async_accept([](error_code){}); + ioc.run(); + ioc.restart(); + BEAST_EXPECT(wsc.is_open()); + BEAST_EXPECT(wss.is_open()); + // invalid 1-byte deflate block in frame + boost::asio::write(wsc.next_layer(), sbuf( + "\xc1\x81\x3a\xa1\x74\x3b\x49")); + error_code ec; + flat_buffer b; + wss.async_read(b, + [&ec](error_code ec_, std::size_t){ ec = ec_; }); + ioc.run(); + BEAST_EXPECTS(ec == zlib::error::end_of_stream, ec.message()); + } + } + + /* Bishop Fox Hybrid Assessment issue 2 + + Happens with permessage-deflate enabled, + and a deflate block with the BFINAL bit set + is encountered in a compressed payload. + */ + void + testIssueBF2() + { + permessage_deflate pmd; + pmd.client_enable = true; + pmd.server_enable = true; + + // read + { + boost::asio::io_context ioc; + stream wsc{ioc}; + stream wss{ioc}; + wsc.set_option(pmd); + wss.set_option(pmd); + wsc.next_layer().connect(wss.next_layer()); + wsc.async_handshake( + "localhost", "/", [](error_code){}); + wss.async_accept([](error_code){}); + ioc.run(); + ioc.restart(); + BEAST_EXPECT(wsc.is_open()); + BEAST_EXPECT(wss.is_open()); + // contains a deflate block with BFINAL set + boost::asio::write(wsc.next_layer(), sbuf( + "\xc1\xf8\xd1\xe4\xcc\x3e\xda\xe4\xcc\x3e" + "\x2b\x1e\x36\xc4\x2b\x1e\x36\xc4\x2b\x1e" + "\x36\x3e\x35\xae\x4f\x54\x18\xae\x4f\x7b" + "\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e" + "\xd1\x1e\x36\xc4\x2b\x1e\x36\xc4\x2b\xe4" + "\x28\x74\x52\x8e\x05\x74\x52\xa1\xcc\x3e" + "\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e" + "\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4\x36\x3e" + "\xd1\xec\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e")); + error_code ec; + flat_buffer b; + wss.read(b, ec); + BEAST_EXPECTS(ec == zlib::error::end_of_stream, ec.message()); + } + + // async read + { + boost::asio::io_context ioc; + stream wsc{ioc}; + stream wss{ioc}; + wsc.set_option(pmd); + wss.set_option(pmd); + wsc.next_layer().connect(wss.next_layer()); + wsc.async_handshake( + "localhost", "/", [](error_code){}); + wss.async_accept([](error_code){}); + ioc.run(); + ioc.restart(); + BEAST_EXPECT(wsc.is_open()); + BEAST_EXPECT(wss.is_open()); + // contains a deflate block with BFINAL set + boost::asio::write(wsc.next_layer(), sbuf( + "\xc1\xf8\xd1\xe4\xcc\x3e\xda\xe4\xcc\x3e" + "\x2b\x1e\x36\xc4\x2b\x1e\x36\xc4\x2b\x1e" + "\x36\x3e\x35\xae\x4f\x54\x18\xae\x4f\x7b" + "\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e" + "\xd1\x1e\x36\xc4\x2b\x1e\x36\xc4\x2b\xe4" + "\x28\x74\x52\x8e\x05\x74\x52\xa1\xcc\x3e" + "\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e" + "\xd1\xe4\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4\x36\x3e" + "\xd1\xec\xcc\x3e\xd1\xe4\xcc\x3e\xd1\xe4" + "\xcc\x3e\xd1\xe4\xcc\x3e")); + error_code ec; + flat_buffer b; + wss.async_read(b, + [&ec](error_code ec_, std::size_t){ ec = ec_; }); + ioc.run(); + BEAST_EXPECTS(ec == zlib::error::end_of_stream, ec.message()); + } + } + void run() override { @@ -1053,6 +1233,8 @@ public: testContHook(); testIssue802(); testIssue807(); + testIssueBF1(); + testIssueBF2(); } }; From 0d8b79abb5d8cc5bd3653b860e069fc2f8e2094b Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Sat, 2 Dec 2017 10:32:29 -0800 Subject: [PATCH 3/4] Install codecov on codecov CI targets only --- .travis.yml | 5 ++--- CHANGELOG.md | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4ab9ff7c..11f2acaa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -43,6 +43,8 @@ matrix: - TOOLSET=gcc - COMPILER=g++-6 - CXXSTD=c++11 + before_install: + - pip install --user https://github.com/codecov/codecov-python/archive/master.zip addons: apt: packages: @@ -118,9 +120,6 @@ matrix: # OSX -before_install: &base_before_install - - pip install --user https://github.com/codecov/codecov-python/archive/master.zip - install: - export BOOST_BRANCH=develop && [ "$TRAVIS_BRANCH" == "master" ] && BOOST_BRANCH=master || true - cd .. diff --git a/CHANGELOG.md b/CHANGELOG.md index ca53dfb4..ec88905d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Version 144-hf1: * Update reports for hybrid assessment * Handle invalid deflate frames +* Install codecov on codecov CI targets only -------------------------------------------------------------------------------- From d52e8290453fe445c8700b4819fe8e4f45633a02 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Sun, 3 Dec 2017 01:14:49 -0800 Subject: [PATCH 4/4] Set version to 144-hf1