Fixes for SIP parser (backport7)

rust/src/sip/detect.rs

@@ -23,9 +23,7 @@ use std::ptr;
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_tx_get_method(
-    tx: &mut SIPTransaction,
-    buffer: *mut *const u8,
-    buffer_len: *mut u32,
+    tx: &mut SIPTransaction, buffer: *mut *const u8, buffer_len: *mut u32,
 ) -> u8 {
     if let Some(ref r) = tx.request {
         let m = &r.method;
@@ -44,9 +42,7 @@ pub unsafe extern "C" fn rs_sip_tx_get_method(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_tx_get_uri(
-    tx: &mut SIPTransaction,
-    buffer: *mut *const u8,
-    buffer_len: *mut u32,
+    tx: &mut SIPTransaction, buffer: *mut *const u8, buffer_len: *mut u32,
 ) -> u8 {
     if let Some(ref r) = tx.request {
         let p = &r.path;
@@ -65,10 +61,7 @@ pub unsafe extern "C" fn rs_sip_tx_get_uri(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_tx_get_protocol(
-    tx: &mut SIPTransaction,
-    buffer: *mut *const u8,
-    buffer_len: *mut u32,
-    direction: u8,
+    tx: &mut SIPTransaction, buffer: *mut *const u8, buffer_len: *mut u32, direction: u8,
 ) -> u8 {
     match direction.into() {
         Direction::ToServer => {
@@ -101,9 +94,7 @@ pub unsafe extern "C" fn rs_sip_tx_get_protocol(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_tx_get_stat_code(
-    tx: &mut SIPTransaction,
-    buffer: *mut *const u8,
-    buffer_len: *mut u32,
+    tx: &mut SIPTransaction, buffer: *mut *const u8, buffer_len: *mut u32,
 ) -> u8 {
     if let Some(ref r) = tx.response {
         let c = &r.code;
@@ -122,9 +113,7 @@ pub unsafe extern "C" fn rs_sip_tx_get_stat_code(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_tx_get_stat_msg(
-    tx: &mut SIPTransaction,
-    buffer: *mut *const u8,
-    buffer_len: *mut u32,
+    tx: &mut SIPTransaction, buffer: *mut *const u8, buffer_len: *mut u32,
 ) -> u8 {
     if let Some(ref r) = tx.response {
         let re = &r.reason;
@@ -143,9 +132,7 @@ pub unsafe extern "C" fn rs_sip_tx_get_stat_msg(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_tx_get_request_line(
-    tx: &mut SIPTransaction,
-    buffer: *mut *const u8,
-    buffer_len: *mut u32,
+    tx: &mut SIPTransaction, buffer: *mut *const u8, buffer_len: *mut u32,
 ) -> u8 {
     if let Some(ref r) = tx.request_line {
         if !r.is_empty() {
@@ -163,9 +150,7 @@ pub unsafe extern "C" fn rs_sip_tx_get_request_line(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_tx_get_response_line(
-    tx: &mut SIPTransaction,
-    buffer: *mut *const u8,
-    buffer_len: *mut u32,
+    tx: &mut SIPTransaction, buffer: *mut *const u8, buffer_len: *mut u32,
 ) -> u8 {
     if let Some(ref r) = tx.response_line {
         if !r.is_empty() {

rust/src/sip/log.rs

@@ -51,4 +51,4 @@ fn log(tx: &SIPTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
 #[no_mangle]
 pub extern "C" fn rs_sip_log_json(tx: &mut SIPTransaction, js: &mut JsonBuilder) -> bool {
     log(tx, js).is_ok()
-}
+}

rust/src/sip/parser.rs

@@ -15,11 +15,11 @@
  * 02110-1301, USA.
  */
 
-// written by Giuseppe Longo <giuseppe@glono.it>
+// written by Giuseppe Longo <giuseppe@glongo.it>
 
-use nom7::bytes::streaming::{take, take_while, take_while1};
+use nom7::bytes::streaming::{tag, take, take_while, take_while1};
 use nom7::character::streaming::{char, crlf};
-use nom7::character::{is_alphabetic, is_alphanumeric, is_space};
+use nom7::character::{is_alphabetic, is_alphanumeric, is_digit, is_space};
 use nom7::combinator::map_res;
 use nom7::sequence::delimited;
 use nom7::{Err, IResult, Needed};
@@ -57,9 +57,13 @@ pub struct Response {
     pub body_len: u16,
 }
 
+/**
+ * Valid tokens and chars are defined in RFC3261:
+ * https://www.rfc-editor.org/rfc/rfc3261#section-25.1
+ */
 #[inline]
 fn is_token_char(b: u8) -> bool {
-    is_alphanumeric(b) || b"!%'*+-._`".contains(&b)
+    is_alphanumeric(b) || b"!%'*+-._`~".contains(&b)
 }
 
 #[inline]
@@ -69,12 +73,12 @@ fn is_method_char(b: u8) -> bool {
 
 #[inline]
 fn is_request_uri_char(b: u8) -> bool {
-    is_alphanumeric(b) || is_token_char(b) || b"~#@:".contains(&b)
+    is_alphanumeric(b) || is_token_char(b) || b"~#@:;=?+&$,/".contains(&b)
 }
 
 #[inline]
 fn is_version_char(b: u8) -> bool {
-    is_alphanumeric(b) || b"./".contains(&b)
+    is_digit(b) || b".".contains(&b)
 }
 
 #[inline]
@@ -107,7 +111,7 @@ pub fn sip_parse_request(oi: &[u8]) -> IResult<&[u8], Request> {
         Request {
             method: method.into(),
             path: path.into(),
-            version: version.into(),
+            version,
             headers,
 
             request_line_len: request_line_len as u16,
@@ -133,7 +137,7 @@ pub fn sip_parse_response(oi: &[u8]) -> IResult<&[u8], Response> {
     Ok((
         bi,
         Response {
-            version: version.into(),
+            version,
             code: code.into(),
             reason: reason.into(),
 
@@ -156,8 +160,10 @@ fn parse_request_uri(i: &[u8]) -> IResult<&[u8], &str> {
 }
 
 #[inline]
-fn parse_version(i: &[u8]) -> IResult<&[u8], &str> {
-    map_res(take_while1(is_version_char), std::str::from_utf8)(i)
+fn parse_version(i: &[u8]) -> IResult<&[u8], String> {
+    let (i, prefix) = map_res(tag("SIP/"), std::str::from_utf8)(i)?;
+    let (i, version) = map_res(take_while1(is_version_char), std::str::from_utf8)(i)?;
+    Ok((i, format!("{}{}", prefix, version)))
 }
 
 #[inline]
@@ -322,4 +328,20 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_parse_invalid_version() {
+        let buf: &[u8] = "HTTP/1.1\r\n".as_bytes();
+
+        // This test must fail if 'HTTP/1.1' is accepted
+        assert!(parse_version(buf).is_err());
+    }
+
+    #[test]
+    fn test_parse_valid_version() {
+        let buf: &[u8] = "SIP/2.0\r\n".as_bytes();
+
+        let (_rem, result) = parse_version(buf).unwrap();
+        assert_eq!(result, "SIP/2.0");
+    }
 }

rust/src/sip/sip.rs

@@ -17,10 +17,10 @@
 
 // written by Giuseppe Longo <giuseppe@glongo.it>
 
-use crate::frames::*;
 use crate::applayer::{self, *};
 use crate::core;
 use crate::core::{AppProto, Flow, ALPROTO_UNKNOWN};
+use crate::frames::*;
 use crate::sip::parser::*;
 use nom7::Err;
 use std;
@@ -96,10 +96,7 @@ impl SIPState {
     }
 
     fn free_tx(&mut self, tx_id: u64) {
-        let tx = self
-            .transactions
-            .iter()
-            .position(|tx| tx.id == tx_id + 1);
+        let tx = self.transactions.iter().position(|tx| tx.id == tx_id + 1);
         debug_assert!(tx.is_some());
         if let Some(idx) = tx {
             let _ = self.transactions.remove(idx);
@@ -149,7 +146,13 @@ impl SIPState {
 
     fn parse_response(&mut self, flow: *const core::Flow, stream_slice: StreamSlice) -> bool {
         let input = stream_slice.as_slice();
-        let _pdu = Frame::new(flow, &stream_slice, input, input.len() as i64, SIPFrameType::Pdu as u8);
+        let _pdu = Frame::new(
+            flow,
+            &stream_slice,
+            input,
+            input.len() as i64,
+            SIPFrameType::Pdu as u8,
+        );
         SCLogDebug!("tc: pdu {:?}", _pdu);
 
         match sip_parse_response(input) {
@@ -224,20 +227,40 @@ fn sip_frames_ts(flow: *const core::Flow, stream_slice: &StreamSlice, r: &Reques
 
 fn sip_frames_tc(flow: *const core::Flow, stream_slice: &StreamSlice, r: &Response) {
     let oi = stream_slice.as_slice();
-    let _f = Frame::new(flow, stream_slice, oi, r.response_line_len as i64, SIPFrameType::ResponseLine as u8);
-    let hi = &oi[r.response_line_len as usize ..];
+    let _f = Frame::new(
+        flow,
+        stream_slice,
+        oi,
+        r.response_line_len as i64,
+        SIPFrameType::ResponseLine as u8,
+    );
+    let hi = &oi[r.response_line_len as usize..];
     SCLogDebug!("tc: response_line {:?}", _f);
-    let _f = Frame::new(flow, stream_slice, hi, r.headers_len as i64, SIPFrameType::ResponseHeaders as u8);
+    let _f = Frame::new(
+        flow,
+        stream_slice,
+        hi,
+        r.headers_len as i64,
+        SIPFrameType::ResponseHeaders as u8,
+    );
     SCLogDebug!("tc: response_headers {:?}", _f);
     if r.body_len > 0 {
-        let bi = &oi[r.body_offset as usize ..];
-        let _f = Frame::new(flow, stream_slice, bi, r.body_len as i64, SIPFrameType::ResponseBody as u8);
+        let bi = &oi[r.body_offset as usize..];
+        let _f = Frame::new(
+            flow,
+            stream_slice,
+            bi,
+            r.body_len as i64,
+            SIPFrameType::ResponseBody as u8,
+        );
         SCLogDebug!("tc: response_body {:?}", _f);
     }
 }
 
 #[no_mangle]
-pub extern "C" fn rs_sip_state_new(_orig_state: *mut std::os::raw::c_void, _orig_proto: AppProto) -> *mut std::os::raw::c_void {
+pub extern "C" fn rs_sip_state_new(
+    _orig_state: *mut std::os::raw::c_void, _orig_proto: AppProto,
+) -> *mut std::os::raw::c_void {
     let state = SIPState::new();
     let boxed = Box::new(state);
     return Box::into_raw(boxed) as *mut _;
@@ -251,8 +274,7 @@ pub extern "C" fn rs_sip_state_free(state: *mut std::os::raw::c_void) {
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_state_get_tx(
-    state: *mut std::os::raw::c_void,
-    tx_id: u64,
+    state: *mut std::os::raw::c_void, tx_id: u64,
 ) -> *mut std::os::raw::c_void {
     let state = cast_pointer!(state, SIPState);
     match state.get_tx_by_id(tx_id) {
@@ -275,8 +297,7 @@ pub unsafe extern "C" fn rs_sip_state_tx_free(state: *mut std::os::raw::c_void,
 
 #[no_mangle]
 pub extern "C" fn rs_sip_tx_get_alstate_progress(
-    _tx: *mut std::os::raw::c_void,
-    _direction: u8,
+    _tx: *mut std::os::raw::c_void, _direction: u8,
 ) -> std::os::raw::c_int {
     1
 }
@@ -285,11 +306,7 @@ static mut ALPROTO_SIP: AppProto = ALPROTO_UNKNOWN;
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_probing_parser_ts(
-    _flow: *const Flow,
-    _direction: u8,
-    input: *const u8,
-    input_len: u32,
-    _rdir: *mut u8,
+    _flow: *const Flow, _direction: u8, input: *const u8, input_len: u32, _rdir: *mut u8,
 ) -> AppProto {
     let buf = build_slice!(input, input_len as usize);
     if sip_parse_request(buf).is_ok() {
@@ -300,11 +317,7 @@ pub unsafe extern "C" fn rs_sip_probing_parser_ts(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_probing_parser_tc(
-    _flow: *const Flow,
-    _direction: u8,
-    input: *const u8,
-    input_len: u32,
-    _rdir: *mut u8,
+    _flow: *const Flow, _direction: u8, input: *const u8, input_len: u32, _rdir: *mut u8,
 ) -> AppProto {
     let buf = build_slice!(input, input_len as usize);
     if sip_parse_response(buf).is_ok() {
@@ -315,23 +328,17 @@ pub unsafe extern "C" fn rs_sip_probing_parser_tc(
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_parse_request(
-    flow: *const core::Flow,
-    state: *mut std::os::raw::c_void,
-    _pstate: *mut std::os::raw::c_void,
-    stream_slice: StreamSlice,
-    _data: *const std::os::raw::c_void,
+    flow: *const core::Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void,
+    stream_slice: StreamSlice, _data: *const std::os::raw::c_void,
 ) -> AppLayerResult {
     let state = cast_pointer!(state, SIPState);
     state.parse_request(flow, stream_slice).into()
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_sip_parse_response(
-    flow: *const core::Flow,
-    state: *mut std::os::raw::c_void,
-    _pstate: *mut std::os::raw::c_void,
-    stream_slice: StreamSlice,
-    _data: *const std::os::raw::c_void,
+    flow: *const core::Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void,
+    stream_slice: StreamSlice, _data: *const std::os::raw::c_void,
 ) -> AppLayerResult {
     let state = cast_pointer!(state, SIPState);
     state.parse_response(flow, stream_slice).into()