From 0cf888a00543e5e4cdf55a12b46b8f9068b63415 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan.bosch@open-xchange.com>
Date: Thu, 11 Jun 2020 14:49:24 +0200
Subject: [PATCH] lib-sieve: plugins: relational: Fix segfault occurring in
mcht_relational_validate().
The segfault happens when this match type is the last argument of the test
command. This situation is not possible in a valid script; positional arguments
are normally present after that, which would prevent the segfault. A variant of
this bug occurs when the match type also has no argument of its own. In either
case the segfault is caused by referring to absent tag arguments, which causes
a NULL dereference.
---
.../plugins/relational/ext-relational-common.c | 10 ++++++----
tests/extensions/relational/errors.svtest | 18 ++++++++++++++++++
.../extensions/relational/errors/syntax.sieve | 8 ++++++++
3 files changed, 32 insertions(+), 4 deletions(-)
create mode 100644 tests/extensions/relational/errors/syntax.sieve
diff --git a/src/lib-sieve/plugins/relational/ext-relational-common.c b/src/lib-sieve/plugins/relational/ext-relational-common.c
index 14b4e0581..a6dd1edfe 100644
--- a/src/lib-sieve/plugins/relational/ext-relational-common.c
+++ b/src/lib-sieve/plugins/relational/ext-relational-common.c
@@ -43,6 +43,7 @@ bool mcht_relational_validate(struct sieve_validator *valdtr,
{
struct sieve_match_type *mcht;
enum relational_match rel_match = REL_MATCH_INVALID;
+ pool_t pool = sieve_ast_argument_pool(ctx->argument);
string_t *rel_match_ident;
/* Check syntax:
@@ -54,14 +55,15 @@ bool mcht_relational_validate(struct sieve_validator *valdtr,
*/
/* Did we get a string in the first place? */
- if ((*arg)->type != SAAT_STRING) {
+ if (*arg == NULL || (*arg)->type != SAAT_STRING) {
sieve_argument_validate_error(
- valdtr, *arg,
+ valdtr, (*arg == NULL ? ctx->argument : *arg),
"the :%s match-type requires a constant string argument being "
"one of \"gt\", \"ge\", \"lt\", \"le\", \"eq\" or \"ne\", "
"but %s was found",
sieve_match_type_name(ctx->match_type),
- sieve_ast_argument_name(*arg));
+ (*arg == NULL ?
+ "none" : sieve_ast_argument_name(*arg)));
return FALSE;
}
@@ -138,7 +140,7 @@ bool mcht_relational_validate(struct sieve_validator *valdtr,
/* Override the actual match type with a parameter-specific one
* FIXME: ugly!
*/
- mcht = p_new(sieve_ast_argument_pool(*arg), struct sieve_match_type, 1);
+ mcht = p_new(pool, struct sieve_match_type, 1);
mcht->object.ext = ctx->match_type->object.ext;
SIEVE_OBJECT_SET_DEF(mcht, rel_match_types[
REL_MATCH_INDEX(ctx->match_type->object.def->code, rel_match)]);
diff --git a/tests/extensions/relational/errors.svtest b/tests/extensions/relational/errors.svtest
index 927475af1..0973b9801 100644
--- a/tests/extensions/relational/errors.svtest
+++ b/tests/extensions/relational/errors.svtest
@@ -4,6 +4,24 @@ require "vnd.dovecot.testsuite";
require "relational";
require "comparator-i;ascii-numeric";
+/*
+ * Syntax errors
+ */
+
+test "Syntax errors" {
+ if test_script_compile "errors/syntax.sieve" {
+ test_fail "compile should have failed";
+ }
+
+ if test_error :count "ne" "6" {
+ test_fail "wrong number of errors reported";
+ }
+}
+
+/*
+ * Validation errors
+ */
+
test "Validation errors" {
if test_script_compile "errors/validation.sieve" {
test_fail "compile should have failed";
diff --git a/tests/extensions/relational/errors/syntax.sieve b/tests/extensions/relational/errors/syntax.sieve
new file mode 100644
index 000000000..c9e818827
--- /dev/null
+++ b/tests/extensions/relational/errors/syntax.sieve
@@ -0,0 +1,8 @@
+require "relational";
+require "comparator-i;ascii-numeric";
+
+# A semicolon in the middle of things
+if address :count "eq" ;comparator "i;ascii-numeric" "to" "3" { }
+
+# A sub-command in the middle of things
+if not address :comparator "i;ascii-numeric" :value e "to" "3" { }
--
GitLab