From 5f3503592861df335655657299f2edcdcdc13b1c Mon Sep 17 00:00:00 2001
From: Philippe Antoine <pantoine@oisf.net>
Date: Thu, 7 Dec 2023 10:32:03 +0100
Subject: [PATCH] filestore: do not try to store a file set to nostore

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.
---
 rust/src/applayer.rs | 26 +++++++++++++++++++++++++-
 src/app-layer-ftp.c  |  2 +-
 src/app-layer-htp.c  |  4 ++--
 src/app-layer-smtp.c |  1 +
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/rust/src/applayer.rs b/rust/src/applayer.rs
index 97aee7363fac..9814143ba5ef 100644
--- a/rust/src/applayer.rs
+++ b/rust/src/applayer.rs
@@ -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) => {
diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c
index 55a947b9f27a..fc64d1eb20fb 100644
--- a/src/app-layer-ftp.c
+++ b/src/app-layer-ftp.c
@@ -1010,7 +1010,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);
 
diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c
index fd979ef7ddc9..e327696ab027 100644
--- a/src/app-layer-htp.c
+++ b/src/app-layer-htp.c
@@ -1448,7 +1448,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;
@@ -1579,7 +1579,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;
     }
diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c
index d1c2ae5a83f9..c03ff75cd070 100644
--- a/src/app-layer-smtp.c
+++ b/src/app-layer-smtp.c
@@ -703,6 +703,7 @@ static int SMTPProcessCommandDATA(
     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;