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(); } };