Skip to content

Commit

Permalink
lib-sieve: Fixed crashes in extension conflict checking.
Browse files Browse the repository at this point in the history 
Registering conflicting commands before conflicts are checked is a bad idea.
Make sure conflicting extensions are not part of remaining validation process.
  • Loading branch information
@stephanbosch
stephanbosch committed Apr 6, 2016
1 parent 228855f commit 905f66a
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 47 deletions.
49 changes: 33 additions & 16 deletions src/lib-sieve/plugins/imap4flags/ext-imapflags.c
View file
Expand Up @@ -107,34 +107,25 @@ static bool ext_imapflags_validator_check_conflict
struct sieve_ast_argument *require_arg,
const struct sieve_extension *other_ext,
bool required);
static bool ext_imapflags_validator_validate
(const struct sieve_extension *ext,
struct sieve_validator *valdtr, void *context,
struct sieve_ast_argument *require_arg,
bool required);

const struct sieve_validator_extension
imapflags_validator_extension = {
.ext = &imapflags_extension,
.check_conflict = ext_imapflags_validator_check_conflict
.check_conflict = ext_imapflags_validator_check_conflict,
.validate = ext_imapflags_validator_validate
};

static bool ext_imapflags_validator_load
(const struct sieve_extension *ext, struct sieve_validator *valdtr)
{
const struct sieve_extension *master_ext =
(const struct sieve_extension *) ext->context;

sieve_validator_extension_register
(valdtr, ext, &imapflags_validator_extension, NULL);

/* Register commands */
sieve_validator_register_command(valdtr, master_ext, &cmd_setflag);
sieve_validator_register_command(valdtr, master_ext, &cmd_addflag);
sieve_validator_register_command(valdtr, master_ext, &cmd_removeflag);

sieve_validator_register_command(valdtr, master_ext, &cmd_mark);
sieve_validator_register_command(valdtr, master_ext, &cmd_unmark);

/* Attach flags side-effect to keep and fileinto actions */
sieve_ext_imap4flags_register_side_effect(valdtr, master_ext, "keep");
sieve_ext_imap4flags_register_side_effect(valdtr, master_ext, "fileinto");

return TRUE;
}

Expand All @@ -158,6 +149,32 @@ static bool ext_imapflags_validator_check_conflict
return TRUE;
}

static bool ext_imapflags_validator_validate
(const struct sieve_extension *ext,
struct sieve_validator *valdtr, void *context ATTR_UNUSED,
struct sieve_ast_argument *require_arg ATTR_UNUSED,
bool required ATTR_UNUSED)
{
const struct sieve_extension *master_ext =
(const struct sieve_extension *) ext->context;

/* No conflicts */

/* Register commands */
sieve_validator_register_command(valdtr, master_ext, &cmd_setflag);
sieve_validator_register_command(valdtr, master_ext, &cmd_addflag);
sieve_validator_register_command(valdtr, master_ext, &cmd_removeflag);

sieve_validator_register_command(valdtr, master_ext, &cmd_mark);
sieve_validator_register_command(valdtr, master_ext, &cmd_unmark);

/* Attach flags side-effect to keep and fileinto actions */
sieve_ext_imap4flags_register_side_effect(valdtr, master_ext, "keep");
sieve_ext_imap4flags_register_side_effect(valdtr, master_ext, "fileinto");

return TRUE;
}

/*
* Interpreter
*/
Expand Down
25 changes: 18 additions & 7 deletions src/lib-sieve/plugins/notify/ext-notify.c
View file
Expand Up @@ -56,10 +56,16 @@ static bool ext_notify_validator_check_conflict
struct sieve_ast_argument *require_arg,
const struct sieve_extension *ext_other,
bool required);
static bool ext_notify_validator_validate
(const struct sieve_extension *ext,
struct sieve_validator *valdtr, void *context,
struct sieve_ast_argument *require_arg,
bool required);

const struct sieve_validator_extension notify_validator_extension = {
.ext = &notify_extension,
.check_conflict = ext_notify_validator_check_conflict
.check_conflict = ext_notify_validator_check_conflict,
.validate = ext_notify_validator_validate
};

static bool ext_notify_validator_load
Expand All @@ -68,11 +74,6 @@ static bool ext_notify_validator_load
/* Register validator extension to check for conflict with enotify */
sieve_validator_extension_register
(valdtr, ext, &notify_validator_extension, NULL);

/* Register new commands */
sieve_validator_register_command(valdtr, ext, &cmd_notify_old);
sieve_validator_register_command(valdtr, ext, &cmd_denotify);

return TRUE;
}

Expand All @@ -94,4 +95,14 @@ static bool ext_notify_validator_check_conflict
return TRUE;
}


static bool ext_notify_validator_validate
(const struct sieve_extension *ext,
struct sieve_validator *valdtr, void *context ATTR_UNUSED,
struct sieve_ast_argument *require_arg ATTR_UNUSED,
bool required ATTR_UNUSED)
{
/* No conflicts: register new commands */
sieve_validator_register_command(valdtr, ext, &cmd_notify_old);
sieve_validator_register_command(valdtr, ext, &cmd_denotify);
return TRUE;
}
52 changes: 30 additions & 22 deletions src/lib-sieve/sieve-validator.c
View file
Expand Up @@ -568,15 +568,23 @@ static bool sieve_validator_extensions_check_conficts
regs = array_get_modifiable(&valdtr->extensions, &count);
for ( i = 0; i < count; i++ ) {
bool required = ext_reg->required && regs[i].required;

if (regs[i].ext == NULL)
continue;
if (regs[i].ext == ext)
continue;
if (!regs[i].loaded)
continue;

/* Check this extension vs other extension */
if ( ext_reg != NULL && ext_reg->valext != NULL &&
ext_reg->valext->check_conflict != NULL ) {
if ( !ext_reg->valext->check_conflict(ext, valdtr,
ext_reg->context, ext_arg, regs[i].ext,
required) )
struct sieve_ast_argument *this_ext_arg =
(ext_arg == NULL ? regs[i].arg : ext_arg);

if ( !ext_reg->valext->check_conflict(ext,
valdtr, ext_reg->context, this_ext_arg,
regs[i].ext, required) )
return FALSE;
}

Expand All @@ -602,16 +610,18 @@ bool sieve_validator_extension_load
struct sieve_validator_extension_reg *reg = NULL;

if ( ext->global && (valdtr->flags & SIEVE_COMPILE_FLAG_NOGLOBAL) != 0 ) {
if ( cmd != NULL && ext_arg != NULL ) {
sieve_argument_validate_error(valdtr, ext_arg,
"%s %s: failed to load Sieve capability `%s': "
"its use is restricted to global scripts",
sieve_command_identifier(cmd), sieve_command_type_name(cmd),
sieve_extension_name(ext));
}
const char *cmd_prefix = (cmd == NULL ? "" :
t_strdup_printf("%s %s: ", sieve_command_identifier(cmd),
sieve_command_type_name(cmd)));
sieve_argument_validate_error(valdtr, ext_arg,
"%sfailed to load Sieve capability `%s': "
"its use is restricted to global scripts",
cmd_prefix, sieve_extension_name(ext));
return FALSE;
}

/* Register extension no matter what and store the
* AST argument registering it */
if ( ext->id >= 0 ) {
reg = array_idx_modifiable
(&valdtr->extensions, (unsigned int) ext->id);
Expand All @@ -622,17 +632,14 @@ bool sieve_validator_extension_load
reg->arg = ext_arg;
}

/* Link extension to AST for use at code generation */
sieve_ast_extension_link(valdtr->ast, ext, reg->required);

if ( extdef->validator_load != NULL &&
!extdef->validator_load(ext, valdtr) ) {
if ( cmd != NULL && ext_arg != NULL ) {
sieve_argument_validate_error(valdtr, ext_arg,
"%s %s: failed to load Sieve capability `%s'",
sieve_command_identifier(cmd), sieve_command_type_name(cmd),
sieve_extension_name(ext));
}
const char *cmd_prefix = (cmd == NULL ? "" :
t_strdup_printf("%s %s: ", sieve_command_identifier(cmd),
sieve_command_type_name(cmd)));
sieve_argument_validate_error(valdtr, ext_arg,
"%sfailed to load Sieve capability `%s'",
cmd_prefix, sieve_extension_name(ext));
return FALSE;
}

Expand All @@ -641,8 +648,9 @@ bool sieve_validator_extension_load
(valdtr, ext_arg, ext) )
return FALSE;

/* Register extension no matter what and store the AST argument registering it
*/
/* Link extension to AST for use at code generation */
sieve_ast_extension_link(valdtr->ast, ext, reg->required);

if ( reg != NULL )
reg->loaded = TRUE;

Expand Down Expand Up @@ -1450,7 +1458,7 @@ static bool sieve_validate_block
/* Validate all 'require'd extensions */
extrs = array_get(&valdtr->extensions, &ext_count);
for ( i = 0; i < ext_count; i++ ) {
if ( extrs[i].valext != NULL
if ( extrs[i].loaded && extrs[i].valext != NULL
&& extrs[i].valext->validate != NULL ) {

if ( !extrs[i].valext->validate(extrs[i].ext,
Expand Down
4 changes: 2 additions & 2 deletions tests/deprecated/notify/errors.svtest
View file
Expand Up @@ -17,7 +17,7 @@ test "Depricated notify extension used with enotify" {
test_fail "compile should have failed";
}

if not test_error :count "eq" :comparator "i;ascii-numeric" "2" {
if not test_error :count "eq" :comparator "i;ascii-numeric" "3" {
test_fail "wrong number of errors reported";
}
}
Expand All @@ -27,7 +27,7 @@ test "Depricated notify extension used with enotify (ihave)" {
test_fail "compile should have failed";
}

if not test_error :count "eq" :comparator "i;ascii-numeric" "2" {
if not test_error :count "eq" :comparator "i;ascii-numeric" "3" {
test_fail "wrong number of errors reported";
}
}
2 changes: 2 additions & 0 deletions tests/deprecated/notify/errors/conflict-ihave.sieve
View file
@@ -1,6 +1,8 @@
require "enotify";
require "ihave";

# 1: Conflict
if ihave "notify" {
# 2: Syntax wrong for enotify (and not skipped in compile)
notify :options "frop@frop.example.org";
}

0 comments on commit 905f66a

Please sign in to comment.