]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
var: Use 16-bit container for type
authorJeff Lucovsky <jlucovsky@oisf.net>
Fri, 1 Nov 2024 14:45:56 +0000 (10:45 -0400)
committerVictor Julien <victor@inliniac.net>
Sun, 9 Mar 2025 06:29:31 +0000 (07:29 +0100)
Issue: 6855: Match sigmatch type field in var and bit structs

Align the size and datatype of type, idx, and next members across:
- FlowVarThreshold
- FlowBit
- FlowVar
- GenericVar
- XBit
- DetectVarList

Note that the FlowVar structure has been intentionally constrained to
match the structure size prior to this commit. To achieve this, the
keylen member was restricted to 8 bits after it was confirmed its value
is checked against a max of 0xff.

src/detect-engine-register.h
src/detect-engine-threshold.c
src/detect-flowvar.c
src/detect-flowvar.h
src/detect-lua-extensions.c
src/detect.h
src/flow-bit.h
src/flow-var.c
src/flow-var.h
src/util-var.h

index c08ac85b872022dc87a1331cbb46ae7f665dc56c..8c1943cd3b6743e97bb1693d11631810a24ada05 100644 (file)
@@ -56,8 +56,7 @@ enum DetectKeywordId {
     DETECT_FLOW,
     /* end prefilter sort */
 
-    /* values used in util-var.c go here, to avoid int overflows
-     * TODO update var logic to use a larger type, see #6855. */
+    /* values used in util-var.c go here, to avoid int overflows */
     DETECT_THRESHOLD,
     DETECT_FLOWBITS,
     DETECT_FLOWVAR,
index 38a7accf745a064610b4f958446826f64ce252e8..a4ccc39b002292fe87e8e03a2a0c11ee61b692ff 100644 (file)
@@ -524,8 +524,8 @@ static void FlowThresholdEntryListFree(FlowThresholdEntryList *list)
 /** struct for storing per flow thresholds. This will be stored in the Flow::flowvar list, so it
  * needs to follow the GenericVar header format. */
 typedef struct FlowVarThreshold_ {
-    uint8_t type;
-    uint8_t pad[7];
+    uint16_t type;
+    uint8_t pad[6];
     struct GenericVar_ *next;
     FlowThresholdEntryList *thresholds;
 } FlowVarThreshold;
index c923be5d0b77ca5ea6ff637825180820886c9c12..f8af531d062e164a0646c09ba43ca5c58c56d001 100644 (file)
@@ -200,9 +200,8 @@ error:
 }
 
 /** \brief Store flowvar in det_ctx so we can exec it post-match */
-int DetectVarStoreMatchKeyValue(DetectEngineThreadCtx *det_ctx,
-        uint8_t *key, uint16_t key_len,
-        uint8_t *buffer, uint16_t len, int type)
+int DetectVarStoreMatchKeyValue(DetectEngineThreadCtx *det_ctx, uint8_t *key, uint16_t key_len,
+        uint8_t *buffer, uint16_t len, uint16_t type)
 {
     DetectVarList *fs = SCCalloc(1, sizeof(*fs));
     if (unlikely(fs == NULL))
@@ -220,9 +219,8 @@ int DetectVarStoreMatchKeyValue(DetectEngineThreadCtx *det_ctx,
 }
 
 /** \brief Store flowvar in det_ctx so we can exec it post-match */
-int DetectVarStoreMatch(DetectEngineThreadCtx *det_ctx,
-        uint32_t idx,
-        uint8_t *buffer, uint16_t len, int type)
+int DetectVarStoreMatch(
+        DetectEngineThreadCtx *det_ctx, uint32_t idx, uint8_t *buffer, uint16_t len, uint16_t type)
 {
     DetectVarList *fs = det_ctx->varlist;
 
index ce0ccdfb591980421db6607be7e35eef816c5171..8e18448156e9b3d208a225473f60c4b86fe9a151 100644 (file)
@@ -38,10 +38,9 @@ typedef struct DetectFlowvarData_ {
 void DetectFlowvarRegister (void);
 
 int DetectFlowvarPostMatchSetup(DetectEngineCtx *de_ctx, Signature *s, uint32_t idx);
-int DetectVarStoreMatch(DetectEngineThreadCtx *,
-        uint32_t, uint8_t *, uint16_t, int);
-int DetectVarStoreMatchKeyValue(DetectEngineThreadCtx *,
-        uint8_t *, uint16_t, uint8_t *, uint16_t, int);
+int DetectVarStoreMatch(DetectEngineThreadCtx *, uint32_t, uint8_t *, uint16_t, uint16_t);
+int DetectVarStoreMatchKeyValue(
+        DetectEngineThreadCtx *, uint8_t *, uint16_t, uint8_t *, uint16_t, uint16_t);
 
 /* For use only by DetectFlowvarProcessList() */
 void DetectVarProcessListInternal(DetectVarList *fs, Flow *f, Packet *p);
index 0dad83749c6724ec4a2f795b6aea2876ca5a61e7..615653ddc739ea76f2ffac89af64d206327baa03 100644 (file)
@@ -129,7 +129,7 @@ static int GetFlowVarByKey(lua_State *luastate, Flow *f, FlowVar **ret_fv)
         LUA_ERROR("key len out of range: max 256");
     }
 
-    FlowVar *fv = FlowVarGetByKey(f, (const uint8_t *)keystr, (uint16_t)keylen);
+    FlowVar *fv = FlowVarGetByKey(f, (const uint8_t *)keystr, (uint8_t)keylen);
     if (fv == NULL) {
         LUA_ERROR("no flow var");
     }
@@ -302,7 +302,7 @@ static int LuaSetFlowvarByKey(lua_State *luastate)
     }
     memcpy(keybuf, keystr, keylen);
     keybuf[keylen] = '\0';
-    FlowVarAddKeyValue(f, keybuf, (uint16_t)keylen, buffer, (uint16_t)len);
+    FlowVarAddKeyValue(f, keybuf, (uint8_t)keylen, buffer, (uint16_t)len);
 
     return 0;
 }
index 5231345288d222a333ab7bd1478a6e4102842740..e48e9e88bb62744d9d700350f4109919fb99d6fc 100644 (file)
@@ -760,10 +760,11 @@ typedef struct DetectReplaceList_ {
 /** list for flowvar store candidates, to be stored from
  *  post-match function */
 typedef struct DetectVarList_ {
+    uint16_t type; /**< type of store candidate POSTMATCH or ALWAYS */
+    uint8_t pad[2];
     uint32_t idx;                       /**< flowvar name idx */
     uint16_t len;                       /**< data len */
     uint16_t key_len;
-    int type;                           /**< type of store candidate POSTMATCH or ALWAYS */
     uint8_t *key;
     uint8_t *buffer;                    /**< alloc'd buffer, may be freed by
                                              post-match, post-non-match */
index e10c58dafde07f5b6558ae127394bfe32a6c54dc..e7fb3f9ecc9c915256a09e071535d49df6ed23f1 100644 (file)
@@ -28,8 +28,8 @@
 #include "util-var.h"
 
 typedef struct FlowBit_ {
-    uint8_t type; /* type, DETECT_FLOWBITS in this case */
-    uint8_t pad[3];
+    uint16_t type; /* type, DETECT_FLOWBITS in this case */
+    uint8_t pad[2];
     uint32_t idx; /* name idx */
     GenericVar *next; /* right now just implement this as a list,
                        * in the long run we have think of something
index 5377607433a0e64f4d1d7f479d873bb7abbddeaa..37e3f267e6693449a1bd5c9676e240e80944972e 100644 (file)
@@ -51,7 +51,7 @@ static void FlowVarUpdateInt(FlowVar *fv, uint32_t value)
  *  \note flow is not locked by this function, caller is
  *        responsible
  */
-FlowVar *FlowVarGetByKey(Flow *f, const uint8_t *key, uint16_t keylen)
+FlowVar *FlowVarGetByKey(Flow *f, const uint8_t *key, FlowVarKeyLenType keylen)
 {
     if (f == NULL)
         return NULL;
@@ -91,7 +91,8 @@ FlowVar *FlowVarGet(Flow *f, uint32_t idx)
 }
 
 /* add a flowvar to the flow, or update it */
-void FlowVarAddKeyValue(Flow *f, uint8_t *key, uint16_t keysize, uint8_t *value, uint16_t size)
+void FlowVarAddKeyValue(
+        Flow *f, uint8_t *key, FlowVarKeyLenType keylen, uint8_t *value, uint16_t size)
 {
     FlowVar *fv = SCCalloc(1, sizeof(FlowVar));
     if (unlikely(fv == NULL))
@@ -103,7 +104,7 @@ void FlowVarAddKeyValue(Flow *f, uint8_t *key, uint16_t keysize, uint8_t *value,
     fv->data.fv_str.value = value;
     fv->data.fv_str.value_len = size;
     fv->key = key;
-    fv->keylen = keysize;
+    fv->keylen = keylen;
     fv->next = NULL;
 
     GenericVarAppend(&f->flowvar, (GenericVar *)fv);
index 4768490e68c65739da02e96ff721a0dee2d06376..d409e54a6bebe32277bdc3b0e81bf0249545b976 100644 (file)
@@ -33,6 +33,7 @@
 #define FLOWVAR_TYPE_STR 1
 #define FLOWVAR_TYPE_INT 2
 
+typedef uint8_t FlowVarKeyLenType;
 /** Struct used to hold the string data type for flowvars */
 typedef struct FlowVarTypeStr {
     uint8_t *value;
@@ -46,9 +47,9 @@ typedef struct FlowVarTypeInt_ {
 
 /** Generic Flowvar Structure */
 typedef struct FlowVar_ {
-    uint8_t type;       /* type, DETECT_FLOWVAR in this case */
+    uint16_t type; /* type, DETECT_FLOWVAR in this case */
     uint8_t datatype;
-    uint16_t keylen;
+    FlowVarKeyLenType keylen;
     uint32_t idx;       /* name idx */
     GenericVar *next;   /* right now just implement this as a list,
                          * in the long run we have think of something
@@ -63,12 +64,13 @@ typedef struct FlowVar_ {
 /** Flowvar Interface API */
 
 void FlowVarAddIdValue(Flow *, uint32_t id, uint8_t *value, uint16_t size);
-void FlowVarAddKeyValue(Flow *f, uint8_t *key, uint16_t keysize, uint8_t *value, uint16_t size);
+void FlowVarAddKeyValue(
+        Flow *f, uint8_t *key, FlowVarKeyLenType keylen, uint8_t *value, uint16_t size);
 
 void FlowVarAddIntNoLock(Flow *, uint32_t, uint32_t);
 void FlowVarAddInt(Flow *, uint32_t, uint32_t);
 FlowVar *FlowVarGet(Flow *, uint32_t);
-FlowVar *FlowVarGetByKey(Flow *f, const uint8_t *key, uint16_t keylen);
+FlowVar *FlowVarGetByKey(Flow *f, const uint8_t *key, FlowVarKeyLenType keylen);
 void FlowVarFree(FlowVar *);
 void FlowVarPrint(GenericVar *);
 
index 4732b46e479cf5304a745b10a0a588e490f77ece..620a923d710f64cb8442ddb27a5944b904a8ae51 100644 (file)
@@ -46,17 +46,16 @@ enum VarTypes {
     VAR_TYPE_IPPAIR_VAR,
 };
 
-/** \todo see ticket #6855. The type field should be 16 bits. */
 typedef struct GenericVar_ {
-    uint8_t type; /**< variable type, uses detection sm_type */
-    uint8_t pad[3];
+    uint16_t type; /**< variable type, uses detection sm_type */
+    uint8_t pad[2];
     uint32_t idx;
     struct GenericVar_ *next;
 } GenericVar;
 
 typedef struct XBit_ {
-    uint8_t type;       /* type, DETECT_XBITS in this case */
-    uint8_t pad[3];
+    uint16_t type; /* type, DETECT_XBITS in this case */
+    uint8_t pad[2];
     uint32_t idx;       /* name idx */
     GenericVar *next;
     uint32_t expire;