app-layer: handle AppLayerTxData being NULL

Http parser can have 'NULL' user data in case of memcap limit getting
reached.

diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c
index 6de88860f584b210f5f46bc00fd6e02086171b4c..badc85c1a3b8154aa2009013ece0a298fdc65dca 100644 (file)
--- a/src/app-layer-parser.c
+++ b/src/app-layer-parser.c
@@ -696,37 +696,25 @@ uint64_t AppLayerParserGetTransactionInspectId(AppLayerParserState *pstate, uint
     SCReturnCT(pstate->inspect_id[direction & STREAM_TOSERVER ? 0 : 1], "uint64_t");
 }
 
-static inline uint64_t GetTxDetectFlags(const uint8_t ipproto, const AppProto alproto, void *tx, const uint8_t dir)
+static inline uint64_t GetTxDetectFlags(AppLayerTxData *txd, const uint8_t dir)
 {
-    uint64_t detect_flags;
-    AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
-    if (txd != NULL) {
-        detect_flags = (dir & STREAM_TOSERVER) ? txd->detect_flags_ts : txd->detect_flags_tc;
-    } else {
-        detect_flags = 0;
-    }
+    uint64_t detect_flags =
+        (dir & STREAM_TOSERVER) ? txd->detect_flags_ts : txd->detect_flags_tc;
     return detect_flags;
 }
 
-static inline void SetTxDetectFlags(const uint8_t ipproto, const AppProto alproto, void *tx, const uint8_t dir, const uint64_t detect_flags)
+static inline void SetTxDetectFlags(AppLayerTxData *txd, const uint8_t dir, const uint64_t detect_flags)
 {
-    AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
-    if (txd != NULL) {
-        if (dir & STREAM_TOSERVER) {
-            txd->detect_flags_ts = detect_flags;
-        } else {
-            txd->detect_flags_tc = detect_flags;
-        }
+    if (dir & STREAM_TOSERVER) {
+        txd->detect_flags_ts = detect_flags;
+    } else {
+        txd->detect_flags_tc = detect_flags;
     }
 }
 
-static inline uint32_t GetTxLogged(const Flow *f, void *alstate, void *tx)
+static inline uint32_t GetTxLogged(AppLayerTxData *txd)
 {
-    AppLayerTxData *txd = AppLayerParserGetTxData(f->proto, f->alproto, tx);
-    if (txd) {
-        return txd->logged.flags;
-    }
-    return 0;
+    return txd->logged.flags;
 }
 
 void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *pstate,
@@ -763,11 +751,12 @@ void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *p
         if (state_progress < state_done_progress)
             break;
 
-        if (tag_txs_as_inspected) {
-            uint64_t detect_flags = GetTxDetectFlags(ipproto, alproto, tx, flags);
+        AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
+        if (txd && tag_txs_as_inspected) {
+            uint64_t detect_flags = GetTxDetectFlags(txd, flags);
             if ((detect_flags & APP_LAYER_TX_INSPECTED_FLAG) == 0) {
                 detect_flags |= APP_LAYER_TX_INSPECTED_FLAG;
-                SetTxDetectFlags(ipproto, alproto, tx, flags, detect_flags);
+                SetTxDetectFlags(txd, flags, detect_flags);
                 SCLogDebug("%p/%"PRIu64" in-order tx is done for direction %s. Flag %016"PRIx64,
                         tx, idx, flags & STREAM_TOSERVER ? "toserver" : "toclient", detect_flags);
             }
@@ -801,15 +790,22 @@ void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *p
             if (state_progress < state_done_progress)
                 break;
 
-            uint64_t detect_flags = GetTxDetectFlags(ipproto, alproto, tx, flags);
-            if ((detect_flags & APP_LAYER_TX_INSPECTED_FLAG) == 0) {
-                detect_flags |= APP_LAYER_TX_INSPECTED_FLAG;
-                SetTxDetectFlags(ipproto, alproto, tx, flags, detect_flags);
-                SCLogDebug("%p/%"PRIu64" out of order tx is done for direction %s. Flag %016"PRIx64,
-                        tx, idx, flags & STREAM_TOSERVER ? "toserver" : "toclient", detect_flags);
-
-                SCLogDebug("%p/%"PRIu64" out of order tx. Update inspect_id? %"PRIu64,
-                        tx, idx, pstate->inspect_id[direction]);
+            /* txd can be NULL for HTTP sessions where the user data alloc failed */
+            AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
+            if (likely(txd)) {
+                uint64_t detect_flags = GetTxDetectFlags(txd, flags);
+                if ((detect_flags & APP_LAYER_TX_INSPECTED_FLAG) == 0) {
+                    detect_flags |= APP_LAYER_TX_INSPECTED_FLAG;
+                    SetTxDetectFlags(txd, flags, detect_flags);
+                    SCLogDebug("%p/%"PRIu64" out of order tx is done for direction %s. Flag %016"PRIx64,
+                            tx, idx, flags & STREAM_TOSERVER ? "toserver" : "toclient", detect_flags);
+
+                    SCLogDebug("%p/%"PRIu64" out of order tx. Update inspect_id? %"PRIu64,
+                            tx, idx, pstate->inspect_id[direction]);
+                    if (pstate->inspect_id[direction]+1 == idx)
+                        pstate->inspect_id[direction] = idx;
+                }
+            } else {
                 if (pstate->inspect_id[direction]+1 == idx)
                     pstate->inspect_id[direction] = idx;
             }
@@ -924,9 +920,10 @@ void AppLayerParserTransactionsCleanup(Flow *f)
             skipped = true;
             goto next;
         }
-        if (has_tx_detect_flags) {
+        AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
+        if (txd && has_tx_detect_flags) {
             if (f->sgh_toserver != NULL) {
-                uint64_t detect_flags_ts = GetTxDetectFlags(ipproto, alproto, tx, STREAM_TOSERVER);
+                uint64_t detect_flags_ts = GetTxDetectFlags(txd, STREAM_TOSERVER);
                 if (!(detect_flags_ts & APP_LAYER_TX_INSPECTED_FLAG)) {
                     SCLogDebug("%p/%"PRIu64" skipping: TS inspect not done: ts:%"PRIx64,
                             tx, i, detect_flags_ts);
@@ -935,7 +932,7 @@ void AppLayerParserTransactionsCleanup(Flow *f)
                 }
             }
             if (f->sgh_toclient != NULL) {
-                uint64_t detect_flags_tc = GetTxDetectFlags(ipproto, alproto, tx, STREAM_TOCLIENT);
+                uint64_t detect_flags_tc = GetTxDetectFlags(txd, STREAM_TOCLIENT);
                 if (!(detect_flags_tc & APP_LAYER_TX_INSPECTED_FLAG)) {
                     SCLogDebug("%p/%"PRIu64" skipping: TC inspect not done: tc:%"PRIx64,
                             tx, i, detect_flags_tc);
@@ -944,8 +941,8 @@ void AppLayerParserTransactionsCleanup(Flow *f)
                 }
             }
         }
-        if (logger_expectation != 0) {
-            LoggerId tx_logged = GetTxLogged(f, alstate, tx);
+        if (txd &&logger_expectation != 0) {
+            LoggerId tx_logged = GetTxLogged(txd);
             if (tx_logged != logger_expectation) {
                 SCLogDebug("%p/%"PRIu64" skipping: logging not done: want:%"PRIx32", have:%"PRIx32,
                         tx, i, logger_expectation, tx_logged);

diff --git a/src/detect.c b/src/detect.c
index 1223720e286b24d42801853f6453a6252fe3f26d..519ae9ea26501aedae7cd151fdd990d43c92346c 100644 (file)
--- a/src/detect.c
+++ b/src/detect.c
@@ -1235,7 +1235,7 @@ static DetectTransaction GetDetectTx(const uint8_t ipproto, const AppProto alpro
 {
     uint64_t detect_flags;
     AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx_ptr);
-    if (txd != NULL) {
+    if (likely(txd != NULL)) {
         detect_flags = (flow_flags & STREAM_TOSERVER) ? txd->detect_flags_ts : txd->detect_flags_tc;
     } else {
         detect_flags = 0;
@@ -1272,7 +1272,7 @@ static inline void StoreDetectFlags(DetectTransaction *tx, const uint8_t flow_fl
         const uint8_t ipproto, const AppProto alproto, const uint64_t detect_flags)
 {
     AppLayerTxData *txd = (AppLayerTxData *)tx->tx_data_ptr;
-    if (txd != NULL) {
+    if (likely(txd != NULL)) {
         if (flow_flags & STREAM_TOSERVER) {
             txd->detect_flags_ts = detect_flags;
         } else {

diff --git a/src/output-tx.c b/src/output-tx.c
index 9446c39a782f98c177dd1c5d783696f46daf7c76..5974168890cd6ca48d8e3185ec745eef20b83f99 100644 (file)
--- a/src/output-tx.c
+++ b/src/output-tx.c
@@ -207,14 +207,13 @@ static TmEcode OutputTxLog(ThreadVars *tv, Packet *p, void *thread_data)
             break;
         void * const tx = ires.tx_ptr;
         tx_id = ires.tx_id;
-        AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
 
-        if (txd) {
-            SCLogDebug("tx %p/%"PRIu64" txd %p: log_flags %x", tx, tx_id, txd, txd->config.log_flags);
-            if (txd->config.log_flags & BIT_U8(CONFIG_TYPE_TX)) {
-                SCLogDebug("SKIP tx %p/%"PRIu64, tx, tx_id);
-                goto next_tx;
-            }
+        AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
+        if (txd == NULL) {
+            /* make sure this tx, which can't be properly logged is skipped */
+            logged = 1;
+            max_id = tx_id;
+            goto next_tx;
         }
 
         if (list[ALPROTO_UNKNOWN] != 0) {
@@ -223,7 +222,13 @@ static TmEcode OutputTxLog(ThreadVars *tv, Packet *p, void *thread_data)
                 goto next_tx;
         }
 
-        LoggerId tx_logged = txd ? txd->logged.flags : 0;
+        SCLogDebug("tx %p/%"PRIu64" txd %p: log_flags %x", tx, tx_id, txd, txd->config.log_flags);
+        if (txd->config.log_flags & BIT_U8(CONFIG_TYPE_TX)) {
+            SCLogDebug("SKIP tx %p/%"PRIu64, tx, tx_id);
+            goto next_tx;
+        }
+
+        LoggerId tx_logged = txd->logged.flags;
         const LoggerId tx_logged_old = tx_logged;
         SCLogDebug("logger: expect %08x, have %08x", logger_expectation, tx_logged);
         if (tx_logged == logger_expectation) {
@@ -295,9 +300,8 @@ next_logger:
         if (tx_logged != tx_logged_old) {
             SCLogDebug("logger: storing %08x (was %08x)",
                 tx_logged, tx_logged_old);
-            if (txd != NULL) {
-                txd->logged.flags |= tx_logged;
-            }
+            DEBUG_VALIDATE_BUG_ON(txd == NULL);
+            txd->logged.flags |= tx_logged;
         }
 
         /* If all loggers logged set a flag and update the last tx_id