HTTP transaction handling improvement

In some cases AppLayerTransactionGetInspectId can return -1, which is
now handled by all it's callers.

Improve logic of selecting which transactions are inspected by the various
HTTP keywords.

src/app-layer-parser.c

@@ -1029,7 +1029,10 @@ error:
     SCReturnInt(-1);
 }
 
-/** \brief get the base transaction id */
+/** \brief get the base transaction id
+ *
+ *  \retval txid or -1 on error
+ */
 int AppLayerTransactionGetInspectId(Flow *f) {
     SCEnter();
 

src/detect-engine-file.c

@@ -196,9 +196,9 @@ int DetectFileInspectHttp(ThreadVars *tv, DetectEngineThreadCtx *det_ctx, Flow *
 
     int r = 0;
     HtpState *htp_state = NULL;
-    size_t idx = 0;
-    size_t start_tx = 0;
-    size_t end_tx = 0;
+    int idx = 0;
+    int start_tx = 0;
+    int end_tx = 0;
     int match = 0;
     FileContainer *ffc;
 
@@ -219,11 +219,15 @@ int DetectFileInspectHttp(ThreadVars *tv, DetectEngineThreadCtx *det_ctx, Flow *
     if (htp_state->connp != NULL && htp_state->connp->conn != NULL)
     {
         start_tx = AppLayerTransactionGetInspectId(f);
+        if (start_tx == -1) {
+            goto end;
+        }
+
         /* tx cnt is incremented after request finishes, so we need to inspect
          * response one before the lowest. */
         if ((flags & STREAM_TOCLIENT) && start_tx > 0)
             start_tx--;
-        end_tx = list_size(htp_state->connp->conn->transactions);
+        end_tx = (int)list_size(htp_state->connp->conn->transactions);
     }
 
     for (idx = start_tx ; idx < end_tx; idx++)

src/detect-engine-hcbd.c

@@ -321,7 +321,7 @@ match:
 static void DetectEngineBufferHttpClientBodies(DetectEngineCtx *de_ctx,
         DetectEngineThreadCtx *det_ctx, Flow *f, HtpState *htp_state)
 {
-    size_t idx = 0;
+    int idx = 0;
     htp_tx_t *tx = NULL;
     int i = 0;
 
@@ -365,8 +365,13 @@ static void DetectEngineBufferHttpClientBodies(DetectEngineCtx *de_ctx,
     }
     memset(det_ctx->hcbd_buffers_len, 0, det_ctx->hcbd_buffers_list_len * sizeof(uint32_t));
 
-    for (idx = AppLayerTransactionGetInspectId(f);
-         i < det_ctx->hcbd_buffers_list_len; idx++, i++) {
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++, i++) {
 
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)

src/detect-engine-hcd.c

@@ -312,9 +312,8 @@ int DetectEngineRunHttpCookieMpm(DetectEngineThreadCtx *det_ctx, Flow *f,
                                  HtpState *htp_state)
 {
     htp_tx_t *tx = NULL;
-    int i;
     uint32_t cnt = 0;
-    size_t idx;
+    int idx;
 
     /* we need to lock because the buffers are not actually true buffers
      * but are ones that point to a buffer given by libhtp */
@@ -331,8 +330,12 @@ int DetectEngineRunHttpCookieMpm(DetectEngineThreadCtx *det_ctx, Flow *f,
     }
 
     idx = AppLayerTransactionGetInspectId(f);
-    int list_size = list_size(htp_state->connp->conn->transactions) - idx;
-    for (i = 0; i < list_size; idx++, i++) {
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++) {
 
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)
@@ -377,8 +380,7 @@ int DetectEngineInspectHttpCookie(DetectEngineCtx *de_ctx,
     int r = 0;
     HtpState *htp_state = NULL;
     htp_tx_t *tx = NULL;
-    int i = 0;
-    size_t idx;
+    int idx;
 
     SCMutexLock(&f->m);
 
@@ -394,8 +396,12 @@ int DetectEngineInspectHttpCookie(DetectEngineCtx *de_ctx,
     }
 
     idx = AppLayerTransactionGetInspectId(f);
-    int list_size = list_size(htp_state->connp->conn->transactions) - idx;
-    for (i = 0; i < list_size; idx++, i++) {
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++) {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)
             continue;

src/detect-engine-hhd.c

@@ -322,7 +322,7 @@ match:
 static void DetectEngineBufferHttpHeaders(DetectEngineThreadCtx *det_ctx, Flow *f,
                                           HtpState *htp_state)
 {
-    size_t idx = 0;
+    int idx = 0;
     htp_tx_t *tx = NULL;
     int i = 0;
 
@@ -362,8 +362,13 @@ static void DetectEngineBufferHttpHeaders(DetectEngineThreadCtx *det_ctx, Flow *
     }
     memset(det_ctx->hhd_buffers_len, 0, det_ctx->hhd_buffers_list_len * sizeof(uint32_t));
 
-    for (idx = AppLayerTransactionGetInspectId(f);
-         i < det_ctx->hhd_buffers_list_len; idx++, i++) {
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++, i++) {
 
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)

src/detect-engine-hmd.c

@@ -312,9 +312,8 @@ int DetectEngineRunHttpMethodMpm(DetectEngineThreadCtx *det_ctx, Flow *f,
                                  HtpState *htp_state)
 {
     htp_tx_t *tx = NULL;
-    int i;
     uint32_t cnt = 0;
-    size_t idx;
+    int idx;
 
     /* we need to lock because the buffers are not actually true buffers
      * but are ones that point to a buffer given by libhtp */
@@ -331,8 +330,12 @@ int DetectEngineRunHttpMethodMpm(DetectEngineThreadCtx *det_ctx, Flow *f,
     }
 
     idx = AppLayerTransactionGetInspectId(f);
-    int list_size = list_size(htp_state->connp->conn->transactions) - idx;
-    for (i = 0; i < list_size; idx++, i++) {
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++) {
 
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL || tx->request_method == NULL)
@@ -370,8 +373,7 @@ int DetectEngineInspectHttpMethod(DetectEngineCtx *de_ctx,
     int r = 0;
     HtpState *htp_state = NULL;
     htp_tx_t *tx = NULL;
-    int i = 0;
-    size_t idx;
+    int idx;
 
     SCMutexLock(&f->m);
 
@@ -387,8 +389,12 @@ int DetectEngineInspectHttpMethod(DetectEngineCtx *de_ctx,
     }
 
     idx = AppLayerTransactionGetInspectId(f);
-    int list_size = list_size(htp_state->connp->conn->transactions) - idx;
-    for (i = 0; i < list_size; idx++, i++) {
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++) {
 
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL || tx->request_method == NULL)

src/detect-engine-hrhd.c

@@ -311,9 +311,8 @@ match:
 int DetectEngineRunHttpRawHeaderMpm(DetectEngineThreadCtx *det_ctx, Flow *f, HtpState *htp_state)
 {
     htp_tx_t *tx = NULL;
-    int i;
     uint32_t cnt = 0;
-    size_t idx;
+    int idx;
 
     /* we need to lock because the buffers are not actually true buffers
      * but are ones that point to a buffer given by libhtp */
@@ -330,8 +329,11 @@ int DetectEngineRunHttpRawHeaderMpm(DetectEngineThreadCtx *det_ctx, Flow *f, Htp
     }
 
     idx = AppLayerTransactionGetInspectId(f);
-    int list_size = list_size(htp_state->connp->conn->transactions) - idx;
-    for (i = 0; i < list_size; idx++, i++) {
+    if (idx == -1) {
+        goto end;
+    }
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++) {
 
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)
@@ -373,8 +375,7 @@ int DetectEngineInspectHttpRawHeader(DetectEngineCtx *de_ctx,
     int r = 0;
     HtpState *htp_state = NULL;
     htp_tx_t *tx = NULL;
-    int i = 0;
-    size_t idx;
+    int idx;
 
     SCMutexLock(&f->m);
 
@@ -390,8 +391,11 @@ int DetectEngineInspectHttpRawHeader(DetectEngineCtx *de_ctx,
     }
 
     idx = AppLayerTransactionGetInspectId(f);
-    int list_size = list_size(htp_state->connp->conn->transactions) - idx;
-    for (i = 0; i < list_size; idx++, i++) {
+    if (idx == -1) {
+        goto end;
+    }
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++) {
 
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)

src/detect-engine-hrud.c

@@ -361,10 +361,14 @@ int DetectEngineRunHttpRawUriMpm(DetectEngineThreadCtx *det_ctx, Flow *f,
         goto end;
     }
 
-    size_t idx = AppLayerTransactionGetInspectId(f);
+    int idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
     htp_tx_t *tx = NULL;
 
-    for ( ; idx < list_size(htp_state->connp->conn->transactions); idx++)
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for ( ; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL || tx->request_uri == NULL)
@@ -422,10 +426,15 @@ int DetectEngineInspectHttpRawUri(DetectEngineCtx *de_ctx,
     SCLogDebug("co->id %"PRIu32, co->id);
 #endif
 
-    size_t idx = AppLayerTransactionGetInspectId(f);
+    int idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
     htp_tx_t *tx = NULL;
 
-    for ( ; idx < list_size(htp_state->connp->conn->transactions); idx++)
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for ( ; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL || tx->request_uri == NULL)

src/detect-engine-uri.c

@@ -457,10 +457,15 @@ int DetectEngineInspectPacketUris(DetectEngineCtx *de_ctx,
     SCLogDebug("co->id %"PRIu32, co->id);
 #endif
 
-    size_t idx = AppLayerTransactionGetInspectId(f);
+    int idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
     htp_tx_t *tx = NULL;
 
-    for ( ; idx < list_size(htp_state->connp->conn->transactions); idx++)
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for ( ; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL || tx->request_uri_normalized == NULL)

src/detect-http-stat-code.c

@@ -103,7 +103,7 @@ int DetectHttpStatCodeMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
     SCEnter();
 
     int ret = 0;
-    size_t idx;
+    int idx;
 
     SCMutexLock(&f->m);
     SCLogDebug("got lock %p", &f->m);
@@ -132,7 +132,13 @@ int DetectHttpStatCodeMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
 
     htp_tx_t *tx = NULL;
 
-    for (idx = 0; idx < list_size(htp_state->connp->conn->transactions); idx++)
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)

src/detect-http-stat-msg.c

@@ -103,7 +103,7 @@ int DetectHttpStatMsgMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
     SCEnter();
 
     int ret = 0;
-    size_t idx;
+    int idx;
 
     SCMutexLock(&f->m);
     SCLogDebug("got lock %p", &f->m);
@@ -132,7 +132,13 @@ int DetectHttpStatMsgMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
 
     htp_tx_t *tx = NULL;
 
-    for (idx = 0; idx < list_size(htp_state->connp->conn->transactions); idx++)
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)

src/detect-pcre.c

@@ -192,7 +192,7 @@ int DetectPcreALDoMatchMethod(DetectEngineThreadCtx *det_ctx, Signature *s,
 
     int ret = 0;
     int toret = 0;
-    size_t idx;
+    int idx;
 
 #define MAX_SUBSTRINGS 30
     int ov[MAX_SUBSTRINGS];
@@ -227,8 +227,13 @@ int DetectPcreALDoMatchMethod(DetectEngineThreadCtx *det_ctx, Signature *s,
 
     htp_tx_t *tx = NULL;
 
-    for (idx = 0;//htp_state->new_in_tx_index;
-         idx < list_size(htp_state->connp->conn->transactions); idx++)
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)
@@ -302,7 +307,7 @@ int DetectPcreALDoMatchCookie(DetectEngineThreadCtx *det_ctx, Signature *s,
 
     int ret = 0;
     int toret = 0;
-    size_t idx;
+    int idx;
 
 #define MAX_SUBSTRINGS 30
     int ov[MAX_SUBSTRINGS];
@@ -337,8 +342,13 @@ int DetectPcreALDoMatchCookie(DetectEngineThreadCtx *det_ctx, Signature *s,
 
     htp_tx_t *tx = NULL;
 
-    for (idx = 0;//htp_state->new_in_tx_index;
-         idx < list_size(htp_state->connp->conn->transactions); idx++)
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL)

src/detect-uricontent.c

@@ -471,7 +471,7 @@ uint32_t DetectUricontentInspectMpm(DetectEngineThreadCtx *det_ctx, Flow *f, Htp
     SCEnter();
 
     uint32_t cnt = 0;
-    size_t idx = 0;
+    int idx = 0;
     htp_tx_t *tx = NULL;
 
     /* locking the flow, we will inspect the htp state */
@@ -483,8 +483,13 @@ uint32_t DetectUricontentInspectMpm(DetectEngineThreadCtx *det_ctx, Flow *f, Htp
         SCReturnUInt(0U);
     }
 
-    for (idx = AppLayerTransactionGetInspectId(f);
-         idx < list_size(htp_state->connp->conn->transactions); idx++)
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL || tx->request_uri_normalized == NULL)
@@ -494,7 +499,7 @@ uint32_t DetectUricontentInspectMpm(DetectEngineThreadCtx *det_ctx, Flow *f, Htp
                 bstr_ptr(tx->request_uri_normalized),
                 bstr_len(tx->request_uri_normalized));
     }
-
+end:
     SCMutexUnlock(&f->m);
     SCReturnUInt(cnt);
 }

src/detect-urilen.c

@@ -110,7 +110,7 @@ int DetectUrilenMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx, Flow *f,
 {
     SCEnter();
     int ret = 0;
-    size_t idx = 0;
+    int idx = 0;
     DetectUrilenData *urilend = (DetectUrilenData *) m->ctx;
 
     HtpState *htp_state = (HtpState *)state;
@@ -122,8 +122,13 @@ int DetectUrilenMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx, Flow *f,
     SCMutexLock(&f->m);
     htp_tx_t *tx = NULL;
 
-    for (idx = 0;//htp_state->new_in_tx_index;
-         idx < list_size(htp_state->connp->conn->transactions); idx++)
+    idx = AppLayerTransactionGetInspectId(f);
+    if (idx == -1) {
+        goto end;
+    }
+
+    int size = (int)list_size(htp_state->connp->conn->transactions);
+    for (; idx < size; idx++)
     {
         tx = list_get(htp_state->connp->conn->transactions, idx);
         if (tx == NULL || tx->request_uri_normalized == NULL)