From: Philippe Antoine <contact@catenacyber.fr>
Date: Tue, 14 Apr 2020 12:24:22 +0000 (+0200)
Subject: smb: resistance against padding evasions
X-Git-Tag: suricata-5.0.4~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=075fbc5c4e2cc203d9f5071d2c69c305bbd66677;p=thirdparty%2Fsuricata.git

smb: resistance against padding evasions

Scenario is use of dummy padding in write AndX request
or other similar commands using a data offset.

Parsing skips now these dummy bytes, and generates one event
---

diff --git a/rust/src/smb/smb1_records.rs b/rust/src/smb/smb1_records.rs
index 35397e5771..1ae9ac50f6 100644
--- a/rust/src/smb/smb1_records.rs
+++ b/rust/src/smb/smb1_records.rs
@@ -78,11 +78,12 @@ named!(pub parse_smb1_write_andx_request_record<Smb1WriteRequestRecord>,
         >>  _remaining: le_u16
         >>  data_len_high: le_u16
         >>  data_len_low: le_u16
-        >>  _data_offset: le_u16
+        >>  data_offset: le_u16
         >>  high_offset: cond!(wct==14,le_u32)
         >>  bcc: le_u16
-        //>>  padding: cond!(data_offset > 32, take!(data_offset - 32))
+        //spec [MS-CIFS].pdf says always take one byte padding
         >>  _padding: cond!(bcc > data_len_low, take!(bcc - data_len_low)) // TODO figure out how this works with data_len_high
+        >>  _padding_evasion: cond!(data_offset > 36+2*(wct as u16), take!(data_offset - (36+2*(wct as u16))))
         >>  file_data: rest
         >> (Smb1WriteRequestRecord {
                 offset: if high_offset != None { ((high_offset.unwrap() as u64) << 32)|(offset as u64) } else { 0 },
@@ -368,7 +369,7 @@ named!(pub parse_smb_trans_response_error_record<SmbRecordTransResponse>,
 
 named!(pub parse_smb_trans_response_regular_record<SmbRecordTransResponse>,
     do_parse!(
-          _wct: le_u8
+          wct: le_u8
        >> _total_param_cnt: le_u16
        >> _total_data_count: le_u16
        >> take!(2) // reserved
@@ -376,12 +377,13 @@ named!(pub parse_smb_trans_response_regular_record<SmbRecordTransResponse>,
        >> _param_offset: le_u16
        >> _param_displacement: le_u16
        >> data_cnt: le_u16
-       >> _data_offset: le_u16
+       >> data_offset: le_u16
        >> _data_displacement: le_u16
        >> _setup_cnt: le_u8
        >> take!(1) // reserved
        >> bcc: le_u16
        >> take!(1) // padding
+       >> _padding_evasion: cond!(data_offset > 36+2*(wct as u16), take!(data_offset - (36+2*(wct as u16))))
        >> data: take!(data_cnt)
        >> (SmbRecordTransResponse {
                 data_cnt,
@@ -489,17 +491,18 @@ pub struct SmbResponseReadAndXRecord<'a> {
 
 named!(pub parse_smb_read_andx_response_record<SmbResponseReadAndXRecord>,
     do_parse!(
-            _wct: le_u8
+            wct: le_u8
         >>  _andx_command: le_u8
         >>  take!(1)    // reserved
         >>  _andx_offset: le_u16
         >>  take!(6)
         >>  data_len_low: le_u16
-        >>  _data_offset: le_u16
+        >>  data_offset: le_u16
         >>  data_len_high: le_u32
         >>  take!(6)    // reserved
         >>  bcc: le_u16
         >>  _padding: cond!(bcc > data_len_low, take!(bcc - data_len_low)) // TODO figure out how this works with data_len_high
+        >>  _padding_evasion: cond!(data_offset > 36+2*(wct as u16), take!(data_offset - (36+2*(wct as u16))))
         >>  file_data: rest
 
         >> (SmbResponseReadAndXRecord {
@@ -669,6 +672,7 @@ named!(pub parse_smb_trans2_request_record<SmbRequestTrans2Record>,
         >>  subcmd: le_u16
         >>  _bcc: le_u16
         >>  _padding: take!(3)
+        //TODO test and use _param_offset and _data_offset
         >>  setup_blob: take!(param_cnt)
         >>  data_blob: take!(data_cnt)