]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
rfb: make sure size calculations do not overflow
authorSascha Steinbiss <satta@debian.org>
Sun, 29 Mar 2020 17:33:29 +0000 (19:33 +0200)
committerVictor Julien <victor@inliniac.net>
Mon, 30 Mar 2020 17:48:03 +0000 (19:48 +0200)
Addresses #3570 by extra checking of calculated size requests.

With the given input, the parser eventually arrived at
parser::parse_failure_reason() which parsed from the remaining four
bytes (describing the string length) that the failure string to follow
would be 4294967295 bytes long. While calculating the total size of the
data to request via AppLayerResult::incomplete(), adding the four bytes
for the parsed but not consumed string length caused the u32 length to
overflow, resulting in a much smaller value triggering the bug condition.

This problem was addressed by more careful checking of values in each step
that could overflow: one subtraction, one addition (which could overflow
the usize length values), and a final check to determine whether the result
still fit into the u32 values required by AppLayerResult::incomplete().
If so, we would safely convert the values and pass them to the result type.
If not, we simply return AppLayerResult::err() but do not erroneously and
silently request the wrong amount.

rust/src/rfb/rfb.rs

index c581a0f2d426dff8c2bd890b63c139dfd989f5ca..5e22d1880d54478f0fffa4fabb15c196e06c5018 100644 (file)
@@ -100,6 +100,20 @@ pub struct RFBState {
     state: parser::RFBGlobalState
 }
 
+#[inline]
+fn handle_incomplete(input: &[u8], current: &[u8], nom_needed: nom::Needed) -> AppLayerResult {
+    if let nom::Needed::Size(needed_size) = nom_needed {
+        if let Some(consumed) = input.len().checked_sub(current.len()) {
+            if let Some(needed) = current.len().checked_add(needed_size) {
+                if consumed <= (std::u32::MAX as usize) && needed <= (std::u32::MAX as usize) {
+                    return AppLayerResult::incomplete(consumed as u32, needed as u32);
+                }
+            }
+        }
+    }
+    return AppLayerResult::err();
+}
+
 impl RFBState {
     pub fn new() -> Self {
         Self {
@@ -183,11 +197,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -214,11 +224,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -239,11 +245,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -263,11 +265,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -318,11 +316,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -350,11 +344,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -386,11 +376,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -411,11 +397,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -442,11 +424,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -464,11 +442,7 @@ impl RFBState {
                             return AppLayerResult::err();
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();
@@ -490,11 +464,7 @@ impl RFBState {
                             }
                         }
                         Err(nom::Err::Incomplete(v)) => {
-                            if let nom::Needed::Size(n) = v {
-                                return AppLayerResult::incomplete((input.len() - current.len()) as u32,
-                                                                  (current.len() + n) as u32);
-                            }
-                            return AppLayerResult::err();
+                            return handle_incomplete(input, current, v);
                         }
                         Err(_) => {
                             return AppLayerResult::err();