From dd9376151cb9c96d402a5924764c5c97dec3082c Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Fri, 1 Nov 2024 10:45:56 -0400 Subject: [PATCH] var: Use 16-bit container for type 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 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 | 3 +-- src/detect-engine-threshold.c | 4 ++-- src/detect-lua-extensions.c | 4 ++-- src/flow-bit.h | 4 ++-- src/flow-var.c | 6 +++--- src/flow-var.h | 9 +++++---- src/util-var.h | 9 ++++----- 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/detect-engine-register.h b/src/detect-engine-register.h index b7a029998555..6d02048019c1 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -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, diff --git a/src/detect-engine-threshold.c b/src/detect-engine-threshold.c index c9ca8fa4a45e..7c67a606417b 100644 --- a/src/detect-engine-threshold.c +++ b/src/detect-engine-threshold.c @@ -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; diff --git a/src/detect-lua-extensions.c b/src/detect-lua-extensions.c index aa2d7a3ae6fc..9dd63f560c4e 100644 --- a/src/detect-lua-extensions.c +++ b/src/detect-lua-extensions.c @@ -158,7 +158,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"); } @@ -331,7 +331,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; } diff --git a/src/flow-bit.h b/src/flow-bit.h index e10c58dafde0..e7fb3f9ecc9c 100644 --- a/src/flow-bit.h +++ b/src/flow-bit.h @@ -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 diff --git a/src/flow-var.c b/src/flow-var.c index a92358f27144..03eb27140733 100644 --- a/src/flow-var.c +++ b/src/flow-var.c @@ -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, keylen_t keylen) { if (f == NULL) return NULL; @@ -91,7 +91,7 @@ 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, keylen_t keylen, uint8_t *value, uint16_t size) { FlowVar *fv = SCCalloc(1, sizeof(FlowVar)); if (unlikely(fv == NULL)) @@ -103,7 +103,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); diff --git a/src/flow-var.h b/src/flow-var.h index 4768490e68c6..d3e2eb8b454e 100644 --- a/src/flow-var.h +++ b/src/flow-var.h @@ -33,6 +33,7 @@ #define FLOWVAR_TYPE_STR 1 #define FLOWVAR_TYPE_INT 2 +typedef uint8_t keylen_t; /** 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; + keylen_t 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,12 @@ 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, keylen_t 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, keylen_t keylen); void FlowVarFree(FlowVar *); void FlowVarPrint(GenericVar *); diff --git a/src/util-var.h b/src/util-var.h index 4732b46e479c..620a923d710f 100644 --- a/src/util-var.h +++ b/src/util-var.h @@ -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;