Skip to content

Commit

Permalink
var: Use 16-bit container for type
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jlucovsky committed Dec 13, 2024
1 parent d11e8a8 commit dd93761
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 20 deletions.
3 changes: 1 addition & 2 deletions src/detect-engine-register.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/detect-engine-threshold.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/detect-lua-extensions.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/flow-bit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/flow-var.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand All @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions src/flow-var.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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 *);

Expand Down
9 changes: 4 additions & 5 deletions src/util-var.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit dd93761

Please sign in to comment.