aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

rfb: make sure size calculations do not overflow

Browse Source 
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.
pull/4762/head
Sascha Steinbiss 6 years ago committed by Victor Julien
parent 2823bc5aed
commit 713c379427
1 changed files with 25 additions and 55 deletions
Show Stats Download Patch File Download Diff File

80
rust/src/rfb/rfb.rs
Unescape Escape View File

@ -100,6 +100,20 @@ pub struct RFBState {
state: parser::RFBGlobalState 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 { impl RFBState {
pub fn new() -> Self { pub fn new() -> Self {
Self { Self {
@ -183,11 +197,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -214,11 +224,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -239,11 +245,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -263,11 +265,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -318,11 +316,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -350,11 +344,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -386,11 +376,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -411,11 +397,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -442,11 +424,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -464,11 +442,7 @@ impl RFBState {
return AppLayerResult::err(); return AppLayerResult::err();
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();
@ -490,11 +464,7 @@ impl RFBState {
} }
} }
Err(nom::Err::Incomplete(v)) => { Err(nom::Err::Incomplete(v)) => {
if let nom::Needed::Size(n) = v { return handle_incomplete(input, current, v);
return AppLayerResult::incomplete((input.len() - current.len()) as u32,
(current.len() + n) as u32);
}
return AppLayerResult::err();
} }
Err(_) => { Err(_) => {
return AppLayerResult::err(); return AppLayerResult::err();

Write Preview
Loading…
Cancel
Save