From de92b3be2e6894c353816c6bd29d2a4768ccb92f Mon Sep 17 00:00:00 2001
From: oatoam <31235342+oatoam@users.noreply.github.com>
Date: Fri, 24 Jun 2022 11:46:17 +0800
Subject: [PATCH] fix unrecoverable Err(Overflow) in websocket frame parser
 (#2790)

---
 actix-http/CHANGES.md      |  6 +++++
 actix-http/src/ws/frame.rs | 49 +++++++++++++++++++++++++++-----------
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md
index 209f86ca..dd6051b8 100644
--- a/actix-http/CHANGES.md
+++ b/actix-http/CHANGES.md
@@ -1,8 +1,14 @@
 # Changes
 
 ## Unreleased - 2022-xx-xx
+### Fixed
+- Websocket parser no longer throws endless overflow errors after receiving an oversized frame. [#2790]
+
+### Changed
 - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency.
 
+[#2790]: https://github.com/actix/actix-web/pull/2790
+
 
 ## 3.1.0 - 2022-06-11
 ### Changed
diff --git a/actix-http/src/ws/frame.rs b/actix-http/src/ws/frame.rs
index 17e34e2b..3659b6c3 100644
--- a/actix-http/src/ws/frame.rs
+++ b/actix-http/src/ws/frame.rs
@@ -17,7 +17,6 @@ impl Parser {
     fn parse_metadata(
         src: &[u8],
         server: bool,
-        max_size: usize,
     ) -> Result<Option<(usize, bool, OpCode, usize, Option<[u8; 4]>)>, ProtocolError> {
         let chunk_len = src.len();
 
@@ -60,20 +59,12 @@ impl Parser {
                 return Ok(None);
             }
             let len = u64::from_be_bytes(TryFrom::try_from(&src[idx..idx + 8]).unwrap());
-            if len > max_size as u64 {
-                return Err(ProtocolError::Overflow);
-            }
             idx += 8;
             len as usize
         } else {
             len as usize
         };
 
-        // check for max allowed size
-        if length > max_size {
-            return Err(ProtocolError::Overflow);
-        }
-
         let mask = if server {
             if chunk_len < idx + 4 {
                 return Ok(None);
@@ -98,11 +89,10 @@ impl Parser {
         max_size: usize,
     ) -> Result<Option<(bool, OpCode, Option<BytesMut>)>, ProtocolError> {
         // try to parse ws frame metadata
-        let (idx, finished, opcode, length, mask) =
-            match Parser::parse_metadata(src, server, max_size)? {
-                None => return Ok(None),
-                Some(res) => res,
-            };
+        let (idx, finished, opcode, length, mask) = match Parser::parse_metadata(src, server)? {
+            None => return Ok(None),
+            Some(res) => res,
+        };
 
         // not enough data
         if src.len() < idx + length {
@@ -112,6 +102,13 @@ impl Parser {
         // remove prefix
         src.advance(idx);
 
+        // check for max allowed size
+        if length > max_size {
+            // drop the payload
+            src.advance(length);
+            return Err(ProtocolError::Overflow);
+        }
+
         // no need for body
         if length == 0 {
             return Ok(Some((finished, opcode, None)));
@@ -339,6 +336,30 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_parse_frame_max_size_recoverability() {
+        let mut buf = BytesMut::new();
+        // The first text frame with length == 2, payload doesn't matter.
+        buf.extend(&[0b0000_0001u8, 0b0000_0010u8, 0b0000_0000u8, 0b0000_0000u8]);
+        // Next binary frame with length == 2 and payload == `[0x1111_1111u8, 0x1111_1111u8]`.
+        buf.extend(&[0b0000_0010u8, 0b0000_0010u8, 0b1111_1111u8, 0b1111_1111u8]);
+
+        assert_eq!(buf.len(), 8);
+        assert!(matches!(
+            Parser::parse(&mut buf, false, 1),
+            Err(ProtocolError::Overflow)
+        ));
+        assert_eq!(buf.len(), 4);
+        let frame = extract(Parser::parse(&mut buf, false, 2));
+        assert!(!frame.finished);
+        assert_eq!(frame.opcode, OpCode::Binary);
+        assert_eq!(
+            frame.payload,
+            Bytes::from(vec![0b1111_1111u8, 0b1111_1111u8])
+        );
+        assert_eq!(buf.len(), 0);
+    }
+
     #[test]
     fn test_ping_frame() {
         let mut buf = BytesMut::new();