From: Josef 'Jeff' Sipek Date: Thu, 24 Sep 2020 17:44:58 +0000 (-0400) Subject: lib: event-filter-parser - Disallow non-equals comparators on non-fields X-Git-Tag: 2.3.13~72 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=23645222de9d5f43d9bf9125e07c643dfc38213c;p=thirdparty%2Fdovecot%2Fcore.git lib: event-filter-parser - Disallow non-equals comparators on non-fields Non-equals comparisons (<, <=, >, and >=) aren't well defined for anything other than fields. Therefore, if we encounter one of these comparators with an event name, category name, or source location, we should error out and avoid confusing the user. --- diff --git a/src/lib/event-filter-parser.y b/src/lib/event-filter-parser.y index 27660955e9..3900b36395 100644 --- a/src/lib/event-filter-parser.y +++ b/src/lib/event-filter-parser.y @@ -42,6 +42,13 @@ static struct event_filter_node *key_value(struct event_filter_parser_state *sta else type = EVENT_FILTER_NODE_TYPE_EVENT_FIELD; + /* only fields support comparators other than EQ */ + if ((type != EVENT_FILTER_NODE_TYPE_EVENT_FIELD) && + (op != EVENT_FILTER_OP_CMP_EQ)) { + state->error = "Only fields support inequality comparisons"; + return NULL; + } + node = p_new(state->pool, struct event_filter_node, 1); node->type = type; node->op = op; @@ -150,7 +157,13 @@ expr : expr AND expr { $$ = logic(state, $1, $3, EVENT_FILTER_OP_AND); } | key_value { $$ = $1; } ; -key_value : key op value { $$ = key_value(state, $1, $3, $2); } +key_value : key op value { + $$ = key_value(state, $1, $3, $2); + if ($$ == NULL) { + yyerror(state, state->error); + YYERROR; + } + } ; key : TOKEN { $$ = $1; } diff --git a/src/lib/test-event-filter-parser.c b/src/lib/test-event-filter-parser.c index 6fb919748b..b489d79055 100644 --- a/src/lib/test-event-filter-parser.c +++ b/src/lib/test-event-filter-parser.c @@ -336,32 +336,43 @@ static void test_event_filter_parser_simple_nesting(void) * Test '' with each possible operator and each possible * quoting of and . Some quotings are not allowed. The keyq * and valueq arguments specify whether the and strings - * should be quoted. + * should be quoted. key_special indicates that the key is *not* a field + * and therefore only the = operator should parse successfully. */ static void generated_single_comparison(const char *name, bool parens, const char *key, enum quoting keyq, + bool key_special, const char *value_in, const char *value_exp, enum quoting valueq) { unsigned int c, q; + bool should_fail; for (c = 0; c < N_ELEMENTS(comparators); c++) { string_t *output = t_str_new(128); - str_append_c(output, '('); - if (keyq != QUOTE_MUST_NOT) + if (key_special && (strcmp(comparators[c], "=") != 0)) { + /* the key is a not a field, only = is allowed */ + str_append(output, "event filter: Only fields support inequality comparisons"); + should_fail = TRUE; + } else { + /* the key is a field, all comparators are allowed */ + str_append_c(output, '('); + if (keyq != QUOTE_MUST_NOT) + str_append_c(output, '"'); + str_append(output, key); + if (keyq != QUOTE_MUST_NOT) + str_append_c(output, '"'); + str_append(output, comparators[c]); str_append_c(output, '"'); - str_append(output, key); - if (keyq != QUOTE_MUST_NOT) + str_append_escaped(output, value_exp, strlen(value_exp)); str_append_c(output, '"'); - str_append(output, comparators[c]); - str_append_c(output, '"'); - str_append_escaped(output, value_exp, strlen(value_exp)); - str_append_c(output, '"'); - str_append_c(output, ')'); + str_append_c(output, ')'); + should_fail = FALSE; + } for (q = 0; q < 4; q++) { const bool qkey = (q & 1) == 1; @@ -396,7 +407,7 @@ static void generated_single_comparison(const char *name, testcase(name, str_c(input), str_c(output), - FALSE); + should_fail); } } } @@ -412,6 +423,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_special[w], QUOTE_MUST_NOT, + TRUE, values_single[v], values_single[v], QUOTE_MAY); @@ -421,6 +433,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_special[w], QUOTE_MUST_NOT, + TRUE, values_multi[v], values_multi[v], QUOTE_MUST); @@ -430,6 +443,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_special[w], QUOTE_MUST_NOT, + TRUE, values_oper[v].in, values_oper[v].out_unquoted, QUOTE_MUST_NOT); @@ -437,6 +451,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_special[w], QUOTE_MUST_NOT, + TRUE, values_oper[v].in, values_oper[v].out_quoted, QUOTE_MUST); @@ -450,6 +465,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_fields_single[w], QUOTE_MAY, + FALSE, values_single[v], values_single[v], QUOTE_MAY); @@ -459,6 +475,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_fields_single[w], QUOTE_MAY, + FALSE, values_multi[v], values_multi[v], QUOTE_MUST); @@ -468,6 +485,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_fields_single[w], QUOTE_MAY, + FALSE, values_oper[v].in, values_oper[v].out_unquoted, QUOTE_MUST_NOT); @@ -475,6 +493,7 @@ static void test_event_filter_parser_generated(bool parens) parens, what_fields_single[w], QUOTE_MAY, + FALSE, values_oper[v].in, values_oper[v].out_quoted, QUOTE_MUST);