]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
filestore: do not try to store a file set to nostore
authorPhilippe Antoine <pantoine@oisf.net>
Thu, 7 Dec 2023 09:32:03 +0000 (10:32 +0100)
committerVictor Julien <victor@inliniac.net>
Sun, 7 Jul 2024 05:10:39 +0000 (07:10 +0200)
Ticket: 6390

This can happen with keyword filestore:both,flow
If one direction does not have a signature group with a filestore,
the file is set to nostore on opening, until a signature in
the other direction tries to set it to store.
Subsequent files will be stored in both directions as flow flags
are now set.

(cherry picked from commit 5f3503592861df335655657299f2edcdcdc13b1c)

rust/src/applayer.rs
src/app-layer-ftp.c
src/app-layer-htp.c
src/app-layer-smtp.c

index 5284a6f6915b7f65d62752d79dfd5fa17415212a..7c00af465c1749c356592c09a4a62785756dc5fd 100644 (file)
@@ -196,11 +196,35 @@ impl AppLayerTxData {
     pub fn update_file_flags(&mut self, state_flags: u16) {
         if (self.file_flags & state_flags) != state_flags {
             SCLogDebug!("updating tx file_flags {:04x} with state flags {:04x}", self.file_flags, state_flags);
-            self.file_flags |= state_flags;
+            let mut nf = state_flags;
+            // With keyword filestore:both,flow :
+            // There may be some opened unclosed file in one direction without filestore
+            // As such it has tx file_flags had FLOWFILE_NO_STORE_TS or TC
+            // But a new file in the other direction may trigger filestore:both,flow
+            // And thus set state_flags FLOWFILE_STORE_TS
+            // If the file was opened without storing it, do not try to store just the end of it
+            if (self.file_flags & FLOWFILE_NO_STORE_TS) != 0 && (state_flags & FLOWFILE_STORE_TS) != 0 {
+                nf &= !FLOWFILE_STORE_TS;
+            }
+            if (self.file_flags & FLOWFILE_NO_STORE_TC) != 0 && (state_flags & FLOWFILE_STORE_TC) != 0 {
+                nf &= !FLOWFILE_STORE_TC;
+            }
+            self.file_flags |= nf;
         }
     }
 }
 
+// need to keep in sync with C flow.h
+pub const FLOWFILE_NO_STORE_TS: u16 = BIT_U16!(2);
+pub const FLOWFILE_NO_STORE_TC: u16 = BIT_U16!(3);
+pub const FLOWFILE_STORE_TS: u16 = BIT_U16!(12);
+pub const FLOWFILE_STORE_TC: u16 = BIT_U16!(13);
+
+#[no_mangle]
+pub unsafe extern "C" fn SCTxDataUpdateFileFlags(txd: &mut AppLayerTxData, state_flags: u16) {
+    txd.update_file_flags(state_flags);
+}
+
 #[macro_export]
 macro_rules!export_tx_data_get {
     ($name:ident, $type:ty) => {
index 9d32f69b769153e6cd4c02a0a488177f012717b2..d1804c61e3baef8c27742482a7e35b284aa5f2af 100644 (file)
@@ -1005,7 +1005,7 @@ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state,
                              ? AppLayerParserStateIssetFlag(pstate, APP_LAYER_PARSER_EOF_TS) != 0
                              : AppLayerParserStateIssetFlag(pstate, APP_LAYER_PARSER_EOF_TC) != 0;
 
-    ftpdata_state->tx_data.file_flags |= ftpdata_state->state_data.file_flags;
+    SCTxDataUpdateFileFlags(&ftpdata_state->tx_data, ftpdata_state->state_data.file_flags);
     if (ftpdata_state->tx_data.file_tx == 0)
         ftpdata_state->tx_data.file_tx = direction & (STREAM_TOSERVER | STREAM_TOCLIENT);
 
index 86fd01ab96ce6643a67ff1dcbcdfa8a1a3d5bae9..7351797046d6133372141f449f80096c2e9bde7b 100644 (file)
@@ -1872,7 +1872,7 @@ static int HTPCallbackRequestBodyData(htp_tx_data_t *d)
     if (tx_ud == NULL) {
         SCReturnInt(HTP_OK);
     }
-    tx_ud->tx_data.file_flags |= hstate->state_data.file_flags;
+    SCTxDataUpdateFileFlags(&tx_ud->tx_data, hstate->state_data.file_flags);
 
     if (!tx_ud->response_body_init) {
         tx_ud->response_body_init = 1;
@@ -2002,7 +2002,7 @@ static int HTPCallbackResponseBodyData(htp_tx_data_t *d)
     if (tx_ud == NULL) {
         SCReturnInt(HTP_OK);
     }
-    tx_ud->tx_data.file_flags |= hstate->state_data.file_flags;
+    SCTxDataUpdateFileFlags(&tx_ud->tx_data, hstate->state_data.file_flags);
     if (!tx_ud->request_body_init) {
         tx_ud->request_body_init = 1;
     }
index 05843a9bb20a908bc0e39e059acbf096f68b67db..5a08c02ce46ae16d2e370c972ef9359d22ba198a 100644 (file)
@@ -832,6 +832,7 @@ static int SMTPProcessCommandDATA(SMTPState *state, SMTPTransaction *tx, Flow *f
     SCEnter();
     DEBUG_VALIDATE_BUG_ON(tx == NULL);
 
+    SCTxDataUpdateFileFlags(&tx->tx_data, state->state_data.file_flags);
     if (!(state->parser_state & SMTP_PARSER_STATE_COMMAND_DATA_MODE)) {
         /* looks like are still waiting for a confirmation from the server */
         return 0;