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,6 +1,7 @@
 Version 144-hf1:
 
 * Update reports for hybrid assessment
+* Handle invalid deflate frames
 
 --------------------------------------------------------------------------------
 

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

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";
         }
     }
 

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);
+                    if(! ec)
-                    // VFALCO See:
+                    {
                         // https://github.com/madler/zlib/issues/280
-                    BOOST_ASSERT(zs.total_out == 0);
+                        if(zs.total_out > 0)
-                    cb_.consume(zs.total_out);
+                            ec = error::partial_deflate_block;
-                    ws_.rd_size_ += zs.total_out;
+                    }
-                    bytes_written_ += zs.total_out;
+                    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);
+                if(! ec)
-                // VFALCO See:
+                {
                     // https://github.com/madler/zlib/issues/280
-                BOOST_ASSERT(zs.total_out == 0);
+                    if(zs.total_out > 0)
-                cb.consume(zs.total_out);
+                        ec = error::partial_deflate_block;
-                rd_size_ += zs.total_out;
+                }
-                bytes_written += zs.total_out;
+                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(

include/boost/beast/zlib/impl/error.ipp

@@ -69,7 +69,7 @@ public:
         switch(static_cast<error>(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";

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

test/beast/websocket/read.cpp

@@ -14,6 +14,8 @@
 
 #include <boost/asio/write.hpp>
 
+#include <boost/beast/core/buffers_to_string.hpp>
+
 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<test::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<test::stream> wsc{ioc};
+            stream<test::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<test::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<test::stream> wsc{ioc};
+            stream<test::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<test::stream> wsc{ioc};
+            stream<test::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<test::stream> wsc{ioc};
+            stream<test::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();
     }
 };