diff --git a/rules/mqtt-events.rules b/rules/mqtt-events.rules index a2ab06ea26..cdabf2984c 100644 --- a/rules/mqtt-events.rules +++ b/rules/mqtt-events.rules @@ -16,3 +16,4 @@ alert mqtt any any -> any any (msg:"SURICATA MQTT unassigned message type (0 or alert mqtt any any -> any any (msg:"SURICATA MQTT too many transactions"; app-layer-event:mqtt.too_many_transactions; classtype:protocol-command-decode; sid:2229009; rev:1;) alert mqtt any any -> any any (msg:"SURICATA MQTT malformed traffic"; app-layer-event:mqtt.malformed_traffic; classtype:protocol-command-decode; sid:2229010; rev:1;) alert mqtt any any -> any any (msg:"SURICATA MQTT too many messages"; app-layer-event:mqtt.too_many_messages; classtype:protocol-command-decode; sid:2229011; rev:1;) +alert mqtt any any -> any any (msg:"SURICATA MQTT too many properties"; app-layer-event:mqtt.too_many_properties; classtype:protocol-command-decode; sid:2229012; rev:1;) diff --git a/rust/src/mqtt/detect.rs b/rust/src/mqtt/detect.rs index 97d75cc4c0..d76053d2a8 100644 --- a/rust/src/mqtt/detect.rs +++ b/rust/src/mqtt/detect.rs @@ -1354,6 +1354,7 @@ mod test { message_id: 1, topics: vec!["foo".to_string(), "baar".to_string()], properties: None, + properties_truncated: false, }), }, Direction::ToServer, @@ -1370,6 +1371,7 @@ mod test { message_id: 1, topics: vec!["fieee".to_string(), "baaaaz".to_string()], properties: None, + properties_truncated: false, }), }); let mut s: *const u8 = std::ptr::null_mut(); @@ -1420,6 +1422,7 @@ mod test { }, ], properties: None, + properties_truncated: false, }), }, Direction::ToServer, @@ -1445,6 +1448,7 @@ mod test { }, ], properties: None, + properties_truncated: false, }), }); let mut s: *const u8 = std::ptr::null_mut(); diff --git a/rust/src/mqtt/mqtt.rs b/rust/src/mqtt/mqtt.rs index e3d942bab2..7018eed1e9 100644 --- a/rust/src/mqtt/mqtt.rs +++ b/rust/src/mqtt/mqtt.rs @@ -48,6 +48,30 @@ static mut MQTT_MAX_TX: usize = 1024; static mut MQTT_MAX_MSGS: usize = 1024; +pub(super) const MQTT_MAX_PROPERTIES: usize = 256; + +fn msg_has_too_many_properties(op: &MQTTOperation) -> bool { + match op { + MQTTOperation::CONNECT(c) => c.properties_truncated, + MQTTOperation::CONNACK(c) => c.properties_truncated, + MQTTOperation::PUBLISH(c) => c.properties_truncated, + MQTTOperation::PUBACK(c) + | MQTTOperation::PUBREC(c) + | MQTTOperation::PUBREL(c) + | MQTTOperation::PUBCOMP(c) => c.properties_truncated, + MQTTOperation::SUBSCRIBE(c) => c.properties_truncated, + MQTTOperation::SUBACK(c) => c.properties_truncated, + MQTTOperation::UNSUBSCRIBE(c) => c.properties_truncated, + MQTTOperation::UNSUBACK(c) => c.properties_truncated, + MQTTOperation::AUTH(c) => c.properties_truncated, + MQTTOperation::DISCONNECT(c) => c.properties_truncated, + MQTTOperation::UNASSIGNED + | MQTTOperation::PINGREQ + | MQTTOperation::PINGRESP + | MQTTOperation::TRUNCATED(_) => false, + } +} + pub(super) static mut ALPROTO_MQTT: AppProto = ALPROTO_UNKNOWN; #[derive(AppLayerFrameType)] @@ -71,6 +95,7 @@ pub enum MQTTEvent { TooManyTransactions, MalformedTraffic, TooManyMessages, + TooManyProperties, } #[derive(Debug)] @@ -88,10 +113,17 @@ pub struct MQTTTransaction { impl MQTTTransaction { pub fn new(msg: MQTTMessage, direction: Direction) -> MQTTTransaction { let mut m = MQTTTransaction::new_empty(direction); - m.msg.push(msg); + m.push_msg(msg); return m; } + fn push_msg(&mut self, msg: MQTTMessage) { + if msg_has_too_many_properties(&msg.op) { + self.tx_data.set_event(MQTTEvent::TooManyProperties as u8); + } + self.msg.push(msg); + } + pub fn new_empty(direction: Direction) -> MQTTTransaction { return MQTTTransaction { tx_id: 0, @@ -308,7 +340,7 @@ impl MQTTState { } MQTTOperation::CONNACK(ref _connack) => { if let Some(tx) = self.get_tx_by_pkt_id(MQTT_CONNECT_PKT_ID) { - tx.msg.push(msg); + tx.push_msg(msg); tx.complete = true; tx.pkt_id = None; self.connected = true; @@ -324,7 +356,7 @@ impl MQTTState { if tx.msg.len() >= unsafe { MQTT_MAX_MSGS } { tx.tx_data.set_event(MQTTEvent::TooManyMessages as u8); } else { - tx.msg.push(msg); + tx.push_msg(msg); } } else { let mut tx = self.new_tx(msg, toclient); @@ -338,7 +370,7 @@ impl MQTTState { } MQTTOperation::PUBACK(ref v) | MQTTOperation::PUBCOMP(ref v) => { if let Some(tx) = self.get_tx_by_pkt_id(v.message_id as u32) { - tx.msg.push(msg); + tx.push_msg(msg); tx.complete = true; tx.pkt_id = None; } else { @@ -353,7 +385,7 @@ impl MQTTState { } MQTTOperation::SUBACK(ref suback) => { if let Some(tx) = self.get_tx_by_pkt_id(suback.message_id as u32) { - tx.msg.push(msg); + tx.push_msg(msg); tx.complete = true; tx.pkt_id = None; } else { @@ -368,7 +400,7 @@ impl MQTTState { } MQTTOperation::UNSUBACK(ref unsuback) => { if let Some(tx) = self.get_tx_by_pkt_id(unsuback.message_id as u32) { - tx.msg.push(msg); + tx.push_msg(msg); tx.complete = true; tx.pkt_id = None; } else { diff --git a/rust/src/mqtt/mqtt_message.rs b/rust/src/mqtt/mqtt_message.rs index 2ba42273ec..8526a3af09 100644 --- a/rust/src/mqtt/mqtt_message.rs +++ b/rust/src/mqtt/mqtt_message.rs @@ -103,6 +103,7 @@ pub struct MQTTConnectData { pub username: Option, pub password: Option>, pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, pub will_properties: Option>, // MQTT 5.0 } @@ -111,6 +112,7 @@ pub struct MQTTConnackData { pub return_code: u8, pub session_present: bool, // MQTT 3.1.1 pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] @@ -119,6 +121,7 @@ pub struct MQTTPublishData { pub message_id: Option, pub message: Vec, pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] @@ -126,6 +129,7 @@ pub struct MQTTMessageIdOnly { pub message_id: u16, pub reason_code: Option, // MQTT 5.0 pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] @@ -139,6 +143,7 @@ pub struct MQTTSubscribeData { pub message_id: u16, pub topics: Vec, pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] @@ -146,6 +151,7 @@ pub struct MQTTSubackData { pub message_id: u16, pub qoss: Vec, pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] @@ -153,25 +159,29 @@ pub struct MQTTUnsubscribeData { pub message_id: u16, pub topics: Vec, pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] pub struct MQTTUnsubackData { pub message_id: u16, pub properties: Option>, // MQTT 5.0 - pub reason_codes: Option>, // MQTT 5.0 + pub properties_truncated: bool, + pub reason_codes: Option>, // MQTT 5.0 } #[derive(Debug)] pub struct MQTTAuthData { pub reason_code: u8, // MQTT 5.0 pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] pub struct MQTTDisconnectData { pub reason_code: Option, // MQTT 5.0 pub properties: Option>, // MQTT 5.0 + pub properties_truncated: bool, } #[derive(Debug)] diff --git a/rust/src/mqtt/parser.rs b/rust/src/mqtt/parser.rs index 1f981b9f0f..486590c474 100644 --- a/rust/src/mqtt/parser.rs +++ b/rust/src/mqtt/parser.rs @@ -18,6 +18,7 @@ // written by Sascha Steinbiss use crate::common::nom7::bits; +use crate::mqtt::mqtt::MQTT_MAX_PROPERTIES; use crate::mqtt::mqtt_message::*; use crate::mqtt::mqtt_property::*; use nom7::bits::streaming::take as take_bits; @@ -99,33 +100,40 @@ fn parse_property(i: &[u8]) -> IResult<&[u8], MQTTProperty> { } #[inline] -fn parse_properties(input: &[u8], precond: bool) -> IResult<&[u8], Option>> { +fn parse_properties( + input: &[u8], precond: bool, +) -> IResult<&[u8], (Option>, bool)> { // do not try to parse anything when precondition is not met if !precond { - return Ok((input, None)); + return Ok((input, (None, false))); } // parse properties length match parse_mqtt_variable_integer(input) { Ok((rem, proplen)) => { if proplen == 0 { // no properties - return Ok((rem, None)); + return Ok((rem, (None, false))); } - // parse properties + // Parse properties, keeping at most MQTT_MAX_PROPERTIES of them let mut props = Vec::::new(); + let mut truncated = false; let (rem, mut newrem) = take(proplen as usize)(rem)?; while !newrem.is_empty() { match parse_property(newrem) { Ok((rem2, val)) => { if val != MQTTProperty::UNKNOWN { - props.push(val); + if props.len() < MQTT_MAX_PROPERTIES { + props.push(val); + } else { + truncated = true; + } } newrem = rem2; } Err(e) => return Err(e), } } - return Ok((rem, Some(props))); + return Ok((rem, (Some(props), truncated))); } Err(e) => return Err(e), } @@ -181,9 +189,10 @@ fn parse_connect(i: &[u8]) -> IResult<&[u8], MQTTConnectData> { let (i, protocol_version) = be_u8(i)?; let (i, rawflags) = be_u8(i)?; let (i, keepalive) = be_u16(i)?; - let (i, properties) = parse_properties(i, protocol_version == 5)?; + let (i, (properties, properties_truncated)) = parse_properties(i, protocol_version == 5)?; let (i, client_id) = parse_mqtt_string(i)?; - let (i, will_properties) = parse_properties(i, protocol_version == 5 && rawflags & 0x4 != 0)?; + let (i, (will_properties, will_properties_truncated)) = + parse_properties(i, protocol_version == 5 && rawflags & 0x4 != 0)?; let (i, will_topic) = cond(rawflags & 0x4 != 0, parse_mqtt_string)(i)?; let (i, will_message) = cond(rawflags & 0x4 != 0, parse_mqtt_binary_data)(i)?; let (i, username) = cond(rawflags & 0x80 != 0, parse_mqtt_string)(i)?; @@ -208,6 +217,7 @@ fn parse_connect(i: &[u8]) -> IResult<&[u8], MQTTConnectData> { password, properties, will_properties, + properties_truncated: properties_truncated || will_properties_truncated, }, )) } @@ -217,13 +227,14 @@ fn parse_connack(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQTTC move |i: &[u8]| { let (i, topic_name_compression_response) = be_u8(i)?; let (i, return_code) = be_u8(i)?; - let (i, properties) = parse_properties(i, protocol_version == 5)?; + let (i, (properties, properties_truncated)) = parse_properties(i, protocol_version == 5)?; Ok(( i, MQTTConnackData { session_present: (topic_name_compression_response & 1) != 0, return_code, properties, + properties_truncated, }, )) } @@ -236,7 +247,8 @@ fn parse_publish( move |i: &[u8]| { let (i, topic) = parse_mqtt_string(i)?; let (i, message_id) = cond(has_id, be_u16)(i)?; - let (message, properties) = parse_properties(i, protocol_version == 5)?; + let (message, (properties, properties_truncated)) = + parse_properties(i, protocol_version == 5)?; Ok(( i, MQTTPublishData { @@ -244,6 +256,7 @@ fn parse_publish( message_id, message: message.to_vec(), properties, + properties_truncated, }, )) } @@ -271,6 +284,7 @@ fn parse_msgidonly(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQT message_id, reason_code: Some(0), properties: None, + properties_truncated: false, }, )); } @@ -286,17 +300,19 @@ fn parse_msgidonly(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQT message_id, reason_code: Some(reason_code), properties: None, + properties_truncated: false, }, )); } match parse_properties(rem, true) { - Ok((rem, properties)) => { + Ok((rem, (properties, properties_truncated))) => { return Ok(( rem, MQTTMessageIdOnly { message_id, reason_code: Some(reason_code), properties, + properties_truncated, }, )); } @@ -320,6 +336,7 @@ fn parse_msgidonly_v3(i: &[u8]) -> IResult<&[u8], MQTTMessageIdOnly> { message_id, reason_code: None, properties: None, + properties_truncated: false, }, )) } @@ -335,7 +352,7 @@ fn parse_subscribe_topic(i: &[u8]) -> IResult<&[u8], MQTTSubscribeTopicData> { fn parse_subscribe(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQTTSubscribeData> { move |i: &[u8]| { let (i, message_id) = be_u16(i)?; - let (i, properties) = parse_properties(i, protocol_version == 5)?; + let (i, (properties, properties_truncated)) = parse_properties(i, protocol_version == 5)?; let (i, topics) = many1(complete(parse_subscribe_topic))(i)?; Ok(( i, @@ -343,6 +360,7 @@ fn parse_subscribe(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQT message_id, topics, properties, + properties_truncated, }, )) } @@ -352,13 +370,15 @@ fn parse_subscribe(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQT fn parse_suback(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQTTSubackData> { move |i: &[u8]| { let (i, message_id) = be_u16(i)?; - let (qoss, properties) = parse_properties(i, protocol_version == 5)?; + let (qoss, (properties, properties_truncated)) = + parse_properties(i, protocol_version == 5)?; Ok(( i, MQTTSubackData { message_id, qoss: qoss.to_vec(), properties, + properties_truncated, }, )) } @@ -370,7 +390,7 @@ fn parse_unsubscribe( ) -> impl Fn(&[u8]) -> IResult<&[u8], MQTTUnsubscribeData> { move |i: &[u8]| { let (i, message_id) = be_u16(i)?; - let (i, properties) = parse_properties(i, protocol_version == 5)?; + let (i, (properties, properties_truncated)) = parse_properties(i, protocol_version == 5)?; let (i, topics) = many0(complete(parse_mqtt_string))(i)?; Ok(( i, @@ -378,6 +398,7 @@ fn parse_unsubscribe( message_id, topics, properties, + properties_truncated, }, )) } @@ -387,7 +408,7 @@ fn parse_unsubscribe( fn parse_unsuback(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQTTUnsubackData> { move |i: &[u8]| { let (i, message_id) = be_u16(i)?; - let (i, properties) = parse_properties(i, protocol_version == 5)?; + let (i, (properties, properties_truncated)) = parse_properties(i, protocol_version == 5)?; let (i, reason_codes) = many0(complete(be_u8))(i)?; Ok(( i, @@ -395,6 +416,7 @@ fn parse_unsuback(protocol_version: u8) -> impl Fn(&[u8]) -> IResult<&[u8], MQTT message_id, properties, reason_codes: Some(reason_codes), + properties_truncated, }, )) } @@ -411,6 +433,7 @@ fn parse_disconnect( MQTTDisconnectData { reason_code: None, properties: None, + properties_truncated: false, }, )); } @@ -423,6 +446,7 @@ fn parse_disconnect( MQTTDisconnectData { reason_code: Some(0), properties: None, + properties_truncated: false, }, )); } @@ -437,16 +461,18 @@ fn parse_disconnect( MQTTDisconnectData { reason_code: Some(0), properties: None, + properties_truncated: false, }, )); } match parse_properties(rem, true) { - Ok((rem, properties)) => { + Ok((rem, (properties, properties_truncated))) => { return Ok(( rem, MQTTDisconnectData { reason_code: Some(reason_code), properties, + properties_truncated, }, )); } @@ -461,12 +487,13 @@ fn parse_disconnect( #[inline] fn parse_auth(i: &[u8]) -> IResult<&[u8], MQTTAuthData> { let (i, reason_code) = be_u8(i)?; - let (i, properties) = parse_properties(i, true)?; + let (i, (properties, properties_truncated)) = parse_properties(i, true)?; Ok(( i, MQTTAuthData { reason_code, properties, + properties_truncated, }, )) } @@ -770,7 +797,7 @@ mod tests { let result = parse_properties(&buf, true); match result { - Ok((remainder, message)) => { + Ok((remainder, (message, _truncated))) => { let res = message.unwrap(); assert_eq!(res[0], MQTTProperty::RECEIVE_MAXIMUM(20)); assert_eq!(remainder.len(), 17); @@ -783,6 +810,7 @@ mod tests { } } } + #[test] fn test_parse_connect() { let buf = [