BUG #1689: fix stack overflow when parsing variables

Suricata crashed when variable (either address or port)
referred to itself or if one created a looped chain of
variables. For instance:

HOME_NET:     "!$EXTERNAL_NET"
EXTERNAL_NET: "!$HOME_NET"

Or:

Var1: "$Var2"
Var2: "$Var3"
Var3: "$Var1"
pull/1905/head
Alexander Gozman 11 years ago
parent 53704ea468
commit 69785f186c

@ -45,6 +45,7 @@
#include "util-debug.h"
#include "util-print.h"
#include "util-var.h"
/* prototypes */
void DetectAddressPrint(DetectAddress *);
@ -915,7 +916,7 @@ error:
*/
static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
DetectAddressHead *gh, DetectAddressHead *ghn,
char *s, int negate)
char *s, int negate, ResolvedVariablesList *var_list)
{
size_t x = 0;
size_t u = 0;
@ -926,6 +927,12 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
char *rule_var_address = NULL;
char *temp_rule_var_address = NULL;
if (AddVariableToResolveList(var_list, s) == -1) {
SCLogError(SC_ERR_INVALID_YAML_CONF_ENTRY, "Found a loop in a address "
"groups declaration. This is likely a misconfiguration.");
goto error;
}
SCLogDebug("s %s negate %s", s, negate ? "true" : "false");
for (u = 0, x = 0; u < size && x < sizeof(address); u++) {
@ -956,7 +963,7 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
/* normal block */
SCLogDebug("normal block");
if (DetectAddressParse2(de_ctx, gh, ghn, address, (negate + n_set) % 2) < 0)
if (DetectAddressParse2(de_ctx, gh, ghn, address, (negate + n_set) % 2, var_list) < 0)
goto error;
} else {
/* negated block
@ -969,7 +976,7 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
DetectAddressHead tmp_gh = { NULL, NULL, NULL };
DetectAddressHead tmp_ghn = { NULL, NULL, NULL };
if (DetectAddressParse2(de_ctx, &tmp_gh, &tmp_ghn, address, 0) < 0)
if (DetectAddressParse2(de_ctx, &tmp_gh, &tmp_ghn, address, 0, var_list) < 0)
goto error;
DetectAddress *tmp_ad;
@ -1037,6 +1044,7 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
SC_RULE_VARS_ADDRESS_GROUPS);
if (rule_var_address == NULL)
goto error;
if (strlen(rule_var_address) == 0) {
SCLogError(SC_ERR_INVALID_SIGNATURE, "variable %s resolved "
"to nothing. This is likely a misconfiguration. "
@ -1044,6 +1052,7 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
"\"!$HOME_NET\" instead of !$HOME_NET. See issue #295.", s);
goto error;
}
SCLogDebug("rule_var_address %s", rule_var_address);
temp_rule_var_address = rule_var_address;
if ((negate + n_set) % 2) {
@ -1053,8 +1062,12 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
snprintf(temp_rule_var_address, strlen(rule_var_address) + 3,
"[%s]", rule_var_address);
}
DetectAddressParse2(de_ctx, gh, ghn, temp_rule_var_address,
(negate + n_set) % 2);
if (DetectAddressParse2(de_ctx, gh, ghn, temp_rule_var_address,
(negate + n_set) % 2, var_list) < 0)
goto error;
d_set = 0;
n_set = 0;
if (temp_rule_var_address != rule_var_address)
@ -1089,6 +1102,7 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
SC_RULE_VARS_ADDRESS_GROUPS);
if (rule_var_address == NULL)
goto error;
if (strlen(rule_var_address) == 0) {
SCLogError(SC_ERR_INVALID_SIGNATURE, "variable %s resolved "
"to nothing. This is likely a misconfiguration. "
@ -1096,6 +1110,7 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
"\"!$HOME_NET\" instead of !$HOME_NET. See issue #295.", s);
goto error;
}
SCLogDebug("rule_var_address %s", rule_var_address);
temp_rule_var_address = rule_var_address;
if ((negate + n_set) % 2) {
@ -1105,8 +1120,9 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
snprintf(temp_rule_var_address, strlen(rule_var_address) + 3,
"[%s]", rule_var_address);
}
if (DetectAddressParse2(de_ctx, gh, ghn, temp_rule_var_address,
(negate + n_set) % 2) < 0) {
(negate + n_set) % 2, var_list) < 0) {
SCLogDebug("DetectAddressParse2 hates us");
goto error;
}
@ -1146,6 +1162,7 @@ static int DetectAddressParse2(const DetectEngineCtx *de_ctx,
return 0;
error:
return -1;
}
@ -1368,6 +1385,8 @@ int DetectAddressTestConfVars(void)
{
SCLogDebug("Testing address conf vars for any misconfigured values");
ResolvedVariablesList var_list = TAILQ_HEAD_INITIALIZER(var_list);
ConfNode *address_vars_node = ConfGetNode("vars.address-groups");
if (address_vars_node == NULL) {
return 0;
@ -1394,7 +1413,10 @@ int DetectAddressTestConfVars(void)
goto error;
}
int r = DetectAddressParse2(NULL, gh, ghn, seq_node->val, /* start with negate no */0);
int r = DetectAddressParse2(NULL, gh, ghn, seq_node->val, /* start with negate no */0, &var_list);
CleanVariableResolveList(&var_list);
if (r < 0) {
SCLogError(SC_ERR_INVALID_YAML_CONF_ENTRY,
"failed to parse address var \"%s\" with value \"%s\". "
@ -1402,6 +1424,8 @@ int DetectAddressTestConfVars(void)
goto error;
}
CleanVariableResolveList(&var_list);
if (DetectAddressIsCompleteIPSpace(ghn)) {
SCLogError(SC_ERR_INVALID_YAML_CONF_ENTRY,
"address var - \"%s\" has the complete IP space negated "
@ -1453,7 +1477,7 @@ int DetectAddressParse(const DetectEngineCtx *de_ctx,
goto error;
}
r = DetectAddressParse2(de_ctx, gh, ghn, str, /* start with negate no */0);
r = DetectAddressParse2(de_ctx, gh, ghn, str, /* start with negate no */0, NULL);
if (r < 0) {
SCLogDebug("DetectAddressParse2 returned %d", r);
goto error;

@ -51,6 +51,7 @@
#include "pkt-var.h"
#include "host.h"
#include "util-profiling.h"
#include "util-var.h"
static int DetectPortCutNot(DetectPort *, DetectPort **);
static int DetectPortCut(DetectEngineCtx *, DetectPort *, DetectPort *,
@ -1025,7 +1026,7 @@ error:
*/
static int DetectPortParseDo(const DetectEngineCtx *de_ctx,
DetectPort **head, DetectPort **nhead,
char *s, int negate)
char *s, int negate, ResolvedVariablesList *var_list)
{
size_t u = 0;
size_t x = 0;
@ -1037,6 +1038,12 @@ static int DetectPortParseDo(const DetectEngineCtx *de_ctx,
char *rule_var_port = NULL;
int r = 0;
if (AddVariableToResolveList(var_list, s) == -1) {
SCLogError(SC_ERR_INVALID_YAML_CONF_ENTRY, "Found a loop in a port "
"groups declaration. This is likely a misconfiguration.");
goto error;
}
SCLogDebug("head %p, *head %p, negate %d", head, *head, negate);
for (u = 0, x = 0; u < size && x < sizeof(address); u++) {
@ -1065,7 +1072,7 @@ static int DetectPortParseDo(const DetectEngineCtx *de_ctx,
SCLogDebug("Parsed port from DetectPortParseDo - %s", address);
x = 0;
r = DetectPortParseDo(de_ctx, head, nhead, address, negate? negate: n_set);
r = DetectPortParseDo(de_ctx, head, nhead, address, negate? negate: n_set, var_list);
if (r == -1)
goto error;
@ -1103,7 +1110,7 @@ static int DetectPortParseDo(const DetectEngineCtx *de_ctx,
temp_rule_var_port = alloc_rule_var_port;
}
r = DetectPortParseDo(de_ctx, head, nhead, temp_rule_var_port,
(negate + n_set) % 2);//negate? negate: n_set);
(negate + n_set) % 2, var_list);//negate? negate: n_set);
if (r == -1)
goto error;
@ -1163,7 +1170,7 @@ static int DetectPortParseDo(const DetectEngineCtx *de_ctx,
temp_rule_var_port = alloc_rule_var_port;
}
r = DetectPortParseDo(de_ctx, head, nhead, temp_rule_var_port,
(negate + n_set) % 2);
(negate + n_set) % 2, var_list);
if (r == -1)
goto error;
@ -1337,6 +1344,8 @@ int DetectPortTestConfVars(void)
{
SCLogDebug("Testing port conf vars for any misconfigured values");
ResolvedVariablesList var_list = TAILQ_HEAD_INITIALIZER(var_list);
ConfNode *port_vars_node = ConfGetNode("vars.port-groups");
if (port_vars_node == NULL) {
return 0;
@ -1361,9 +1370,15 @@ int DetectPortTestConfVars(void)
goto error;
}
int r = DetectPortParseDo(NULL, &gh, &ghn, seq_node->val, /* start with negate no */0);
int r = DetectPortParseDo(NULL, &gh, &ghn, seq_node->val, /* start with negate no */0, &var_list);
CleanVariableResolveList(&var_list);
if (r < 0) {
DetectPortCleanupList(gh);
SCLogError(SC_ERR_INVALID_YAML_CONF_ENTRY,
"failed to parse port var \"%s\" with value \"%s\". "
"Please check it's syntax", seq_node->name, seq_node->val);
goto error;
}
@ -1409,7 +1424,7 @@ int DetectPortParse(const DetectEngineCtx *de_ctx,
/* negate port list */
DetectPort *nhead = NULL;
r = DetectPortParseDo(de_ctx, head, &nhead, str,/* start with negate no */0);
r = DetectPortParseDo(de_ctx, head, &nhead, str,/* start with negate no */0, NULL);
if (r < 0)
goto error;
@ -2786,7 +2801,7 @@ static int PortTestMatchDoubleNegation(void)
int result = 0;
DetectPort *head = NULL, *nhead = NULL;
if (DetectPortParseDo(NULL, &head, &nhead, "![!80]", 0) == -1)
if (DetectPortParseDo(NULL, &head, &nhead, "![!80]", 0, NULL) == -1)
return result;
result = (head != NULL);

@ -128,3 +128,43 @@ void GenericVarRemove(GenericVar **list, GenericVar *gv)
listgv = listgv->next;
}
}
// Checks if a variable is already in a resolve list and if it's not, adds it.
int AddVariableToResolveList(ResolvedVariablesList *list, char *var)
{
ResolvedVariable *p_item;
if (list == NULL || var == NULL)
return 0;
TAILQ_FOREACH(p_item, list, next) {
if (!strcmp(p_item->var_name, var)) {
return -1;
}
}
p_item = SCMalloc(sizeof(ResolvedVariable));
if (unlikely(p_item == NULL)) {
return -1;
}
strlcpy(p_item->var_name, var, sizeof(p_item->var_name) - 1);
TAILQ_INSERT_TAIL(list, p_item, next);
return 0;
}
void CleanVariableResolveList(ResolvedVariablesList *var_list)
{
if (var_list == NULL) {
return;
}
ResolvedVariable *p_item;
while ((p_item = TAILQ_FIRST(var_list))) {
TAILQ_REMOVE(var_list, p_item, next);
SCFree(p_item);
}
}

@ -57,9 +57,21 @@ typedef struct XBit_ {
uint32_t expire;
} XBit;
// A list of variables we try to resolve while parsing configuration file.
// Helps to detect recursive declarations.
typedef struct ResolvedVariable_ {
char var_name[256];
TAILQ_ENTRY(ResolvedVariable_) next;
} ResolvedVariable;
typedef TAILQ_HEAD(, ResolvedVariable_) ResolvedVariablesList;
void GenericVarFree(GenericVar *);
void GenericVarAppend(GenericVar **, GenericVar *);
void GenericVarRemove(GenericVar **, GenericVar *);
int AddVariableToResolveList(ResolvedVariablesList *list, char *var);
void CleanVariableResolveList(ResolvedVariablesList *var_list);
#endif /* __UTIL_VAR_H__ */

Loading…
Cancel
Save