Next/60x/20220419/v5

.github/workflows/builds.yml

@@ -152,7 +152,7 @@ jobs:
           path: ~/.cargo/registry
           key: cargo-registry
 
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
 
       # Prebuild check for duplicat SIDs
       - name: Check for duplicate SIDs
@@ -359,7 +359,7 @@ jobs:
                 sudo \
                 which \
                 zlib-devel
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -435,7 +435,7 @@ jobs:
                 sudo \
                 which \
                 zlib-devel
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -502,7 +502,7 @@ jobs:
                 zlib1g \
                 zlib1g-dev \
                 exuberant-ctags
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -567,7 +567,7 @@ jobs:
                 zlib1g \
                 zlib1g-dev \
                 exuberant-ctags
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -692,7 +692,7 @@ jobs:
                 zlib1g \
                 zlib1g-dev \
                 exuberant-ctags
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -777,7 +777,7 @@ jobs:
         run: |
           add-apt-repository -y ppa:npalix/coccinelle
           apt -y install coccinelle
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -856,7 +856,7 @@ jobs:
                 zlib1g \
                 zlib1g-dev
       - run: echo "$HOME/.cargo/bin" >> $GITHUB_PATH
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -978,7 +978,7 @@ jobs:
       - name: Install Rust
         run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain $RUST_VERSION_KNOWN -y
       - run: echo "$HOME/.cargo/bin" >> $GITHUB_PATH
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -1042,7 +1042,7 @@ jobs:
       - name: Install Rust
         run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain $RUST_VERSION_KNOWN -y
       - run: echo "$HOME/.cargo/bin" >> $GITHUB_PATH
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep
@@ -1098,7 +1098,7 @@ jobs:
         run: cargo install --force --debug --version 0.14.1 cbindgen
       - run: echo "$HOME/.cargo/bin" >> $GITHUB_PATH
       - run: pip3 install PyYAML
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - name: Downloading prep archive
         uses: actions/download-artifact@v2
         with:
@@ -1121,7 +1121,7 @@ jobs:
       run:
         shell: msys2 {0}
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: msys2/setup-msys2@v2
         with:
           msystem: MINGW64
@@ -1131,7 +1131,7 @@ jobs:
       # preinstalled one to be picked up by configure
       - name: cbindgen
         run: cargo install --root /usr --force --debug --version 0.14.1 cbindgen
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
       - uses: actions/download-artifact@v2
         with:
           name: prep

.github/workflows/commits.yml

@@ -3,15 +3,20 @@ name: commit-check
 on:
   - pull_request
 
+permissions: read-all
+
+env:
+  DEBIAN_FRONTEND: "noninteractive"
+
 jobs:
 
   check-commits:
     name: Commit Check
     runs-on: ubuntu-latest
-    container: ubuntu:18.04
+    container: ubuntu:20.04
     steps:
       - name: Caching ~/.cargo
-        uses: actions/cache@v1
+        uses: actions/cache@937d24475381cd9c75ae6db12cb4e79714b926ed
         with:
           path: ~/.cargo
           key: commit-check-cargo
@@ -42,7 +47,7 @@ jobs:
                 libhiredis-dev \
                 libjansson-dev \
                 libevent-dev \
-                libevent-pthreads-2.1.6 \
+                libevent-pthreads-2.1-7 \
                 libjansson-dev \
                 libpython2.7 \
                 libssl-dev \
@@ -59,7 +64,11 @@ jobs:
       - name: Install cbindgen
         run: cargo install cbindgen
       - run: echo $PATH
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
+        with:
+          fetch-depth: 0
+      # The action above is supposed to do this for us, but it doesn't appear to stick.
+      - run: /usr/bin/git config --global --add safe.directory /__w/suricata/suricata
       - run: git fetch
       - run: git clone https://github.com/OISF/libhtp -b 0.5.x
       - name: Building all commits

.github/workflows/formatting.yml

@@ -8,19 +8,24 @@ on:
       - 'master-*'
   pull_request:
 
+permissions: read-all
+
+env:
+  DEBIAN_FRONTEND: "noninteractive"
+
 jobs:
 
   # Checking for correct formatting of branch for C code changes
   check-formatting:
     name: Formatting Check (clang 9)
-    runs-on: ubuntu-18.04
-    container: ubuntu:18.04
-    continue-on-error: true
+    runs-on: ubuntu-20.04
+    container: ubuntu:20.04
+    continue-on-error: false
     steps:
 
       # Cache Rust stuff.
       - name: Cache cargo registry
-        uses: actions/cache@v1
+        uses: actions/cache@937d24475381cd9c75ae6db12cb4e79714b926ed
         with:
           path: ~/.cargo/registry
           key: cargo-registry
@@ -81,15 +86,18 @@ jobs:
       # My patience simply ran too short to keep on looking. See follow-on
       # action to manually fix this up.
       - name: Checkout - might be merge commit!
-        uses: actions/checkout@v1
+        uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748
+        with:
+          fetch-depth: 0
         # Use last commit of branch, not potential merge commit!
         #
         # This works perfectly well on pull requests within forked repos, but
         # not for pull requests from forks to the OISF repo as the latter one
         # does not know the branch (from the forked repo). Argh.
 #        with:
 #          ref: ${{ github.head_ref }} # check out branch
-
+      # The action above is supposed to do this for us, but it doesn't appear to stick.
+      - run: /usr/bin/git config --global --add safe.directory /__w/suricata/suricata
       # Manually ignore the merge commit as none of the with/ref things tried
       # with actions/checkout seemed to work for pull requests from forks into
       # the OISF repo.

rules/mqtt-events.rules

@@ -13,3 +13,4 @@ alert mqtt any any -> any any (msg:"SURICATA MQTT message seen before CONNECT/CO
 alert mqtt any any -> any any (msg:"SURICATA MQTT invalid QOS level"; app-layer-event:mqtt.invalid_qos_level; classtype:protocol-command-decode; sid:2229006; rev:1;)
 alert mqtt any any -> any any (msg:"SURICATA MQTT missing message ID"; app-layer-event:mqtt.missing_msg_id; classtype:protocol-command-decode; sid:2229007; rev:1;)
 alert mqtt any any -> any any (msg:"SURICATA MQTT unassigned message type (0 or >15)"; app-layer-event:mqtt.unassigned_msg_type; classtype:protocol-command-decode; sid:2229008; 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;)

rules/smb-events.rules

@@ -16,3 +16,5 @@ alert smb any any -> any any (msg:"SURICATA SMB malformed NTLMSSP record"; flow:
 alert smb any any -> any any (msg:"SURICATA SMB malformed request dialects"; flow:to_server; app-layer-event:smb.negotiate_malformed_dialects; classtype:protocol-command-decode; sid:2225005; rev:1;)
 
 alert smb any any -> any any (msg:"SURICATA SMB file overlap"; app-layer-event:smb.file_overlap; classtype:protocol-command-decode; sid:2225006; rev:1;)
+alert smb any any -> any any (msg:"SURICATA SMB wrong direction"; app-layer-event:smb.response_to_server; classtype:protocol-command-decode; sid:2225007; rev:1;)
+alert smb any any -> any any (msg:"SURICATA SMB wrong direction"; app-layer-event:smb.request_to_client; classtype:protocol-command-decode; sid:2225008; rev:1;)

rust/src/mqtt/mqtt.rs

@@ -51,6 +51,7 @@ pub enum MQTTEvent {
     InvalidQosLevel,
     MissingMsgId,
     UnassignedMsgtype,
+    MalformedTraffic,
 }
 
 #[derive(Debug)]
@@ -70,7 +71,13 @@ pub struct MQTTTransaction {
 
 impl MQTTTransaction {
     pub fn new(msg: MQTTMessage) -> MQTTTransaction {
-        let mut m = MQTTTransaction {
+        let mut m = MQTTTransaction::new_empty();
+        m.msg.push(msg);
+        return m;
+    }
+
+    pub fn new_empty() -> MQTTTransaction {
+        return MQTTTransaction {
             tx_id: 0,
             pkt_id: None,
             complete: false,
@@ -82,8 +89,6 @@ impl MQTTTransaction {
             events: std::ptr::null_mut(),
             tx_data: applayer::AppLayerTxData::new(),
         };
-        m.msg.push(msg);
-        return m;
     }
 
     pub fn free(&mut self) {
@@ -427,25 +432,25 @@ impl MQTTState {
 
 
         while current.len() > 0 {
-            let mut skipped = false;
             SCLogDebug!("request: handling {}", current.len());
             match parse_message(current, self.protocol_version, self.max_msg_len) {
                 Ok((rem, msg)) => {
                     SCLogDebug!("request msg {:?}", msg);
                     if let MQTTOperation::TRUNCATED(ref trunc) = msg.op {
                         SCLogDebug!("found truncated with skipped {} current len {}", trunc.skipped_length, current.len());
                         if trunc.skipped_length >= current.len() {
-                            skipped = true;
                             self.skip_request = trunc.skipped_length - current.len();
+                            self.handle_msg(msg, true);
+                            return AppLayerResult::ok();
                         } else {
+                            consumed += trunc.skipped_length;
                             current = &current[trunc.skipped_length..];
+                            self.handle_msg(msg, true);
                             self.skip_request = 0;
+                            continue;
                         }
                     }
                     self.handle_msg(msg, false);
-                    if skipped {
-                        return AppLayerResult::ok();
-                    }
                     consumed += current.len() - rem.len();
                     current = rem;
                 }
@@ -454,6 +459,7 @@ impl MQTTState {
                         return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32);
                 }
                 Err(_) => {
+                    self.set_event_notx(MQTTEvent::MalformedTraffic, false);
                     return AppLayerResult::err();
                 }
             }
@@ -483,26 +489,26 @@ impl MQTTState {
         }
 
         while current.len() > 0 {
-            let mut skipped = false;
             SCLogDebug!("response: handling {}", current.len());
             match parse_message(current, self.protocol_version, self.max_msg_len as usize) {
                 Ok((rem, msg)) => {
                     SCLogDebug!("response msg {:?}", msg);
                     if let MQTTOperation::TRUNCATED(ref trunc) = msg.op {
                         SCLogDebug!("found truncated with skipped {} current len {}", trunc.skipped_length, current.len());
                         if trunc.skipped_length >= current.len() {
-                            skipped = true;
                             self.skip_response = trunc.skipped_length - current.len();
+                            self.handle_msg(msg, true);
+                            SCLogDebug!("skip_response now {}", self.skip_response);
+                            return AppLayerResult::ok();
                         } else {
+                            consumed += trunc.skipped_length;
                             current = &current[trunc.skipped_length..];
+                            self.handle_msg(msg, true);
                             self.skip_response = 0;
+                            continue;
                         }
-                        SCLogDebug!("skip_response now {}", self.skip_response);
                     }
                     self.handle_msg(msg, true);
-                    if skipped {
-                        return AppLayerResult::ok();
-                    }
                     consumed += current.len() - rem.len();
                     current = rem;
                 }
@@ -511,6 +517,7 @@ impl MQTTState {
                     return AppLayerResult::incomplete(consumed as u32, (current.len() + 1) as u32);
                 }
                 Err(_) => {
+                    self.set_event_notx(MQTTEvent::MalformedTraffic, true);
                     return AppLayerResult::err();
                 }
             }
@@ -544,6 +551,20 @@ impl MQTTState {
 
         return None;
     }
+
+    fn set_event_notx(&mut self, event: MQTTEvent, toclient: bool) {
+        let mut tx = MQTTTransaction::new_empty();
+        self.tx_id += 1;
+        tx.tx_id = self.tx_id;
+        if toclient {
+            tx.toclient = true;
+        } else {
+            tx.toserver = true;
+        }
+        tx.complete = true;
+        MQTTState::set_event(&mut tx, event);
+        self.transactions.push(tx);
+    }
 }
 
 // C exports.
@@ -717,6 +738,7 @@ pub extern "C" fn rs_mqtt_state_get_event_info_by_id(event_id: std::os::raw::c_i
             MQTTEvent::InvalidQosLevel     => { "invalid_qos_level\0" },
             MQTTEvent::MissingMsgId        => { "missing_msg_id\0" },
             MQTTEvent::UnassignedMsgtype   => { "unassigned_msg_type\0" },
+            MQTTEvent::MalformedTraffic    => { "malformed_traffic\0" },
         };
         unsafe{
             *event_name = estr.as_ptr() as *const std::os::raw::c_char;
@@ -748,6 +770,7 @@ pub extern "C" fn rs_mqtt_state_get_event_info(event_name: *const std::os::raw::
                 "invalid_qos_level"    => MQTTEvent::InvalidQosLevel as i32,
                 "missing_msg_id"       => MQTTEvent::MissingMsgId as i32,
                 "unassigned_msg_type"  => MQTTEvent::UnassignedMsgtype as i32,
+                "malformed_traffic"    => MQTTEvent::MalformedTraffic as i32,
                 _                      => -1, // unknown event
             }
         },