From 9804935437c11f795c0ec8af65623e3d2f3fd0a2 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan.bosch@open-xchange.com>
Date: Fri, 30 Aug 2024 01:25:25 +0200
Subject: [PATCH] lib-sieve: plugins: include: Fix crash occurring when more
than one optional include is present
---
src/lib-sieve/plugins/include/cmd-include.c | 6 +-
.../plugins/include/ext-include-binary.c | 92 +++++++++++++------
.../plugins/include/ext-include-binary.h | 11 ++-
.../plugins/include/ext-include-common.c | 40 +++++---
.../plugins/include/ext-include-common.h | 4 +-
src/lib-sieve/plugins/include/ext-include.c | 2 +-
.../include/execute/optional-missing.sieve | 4 +
tests/extensions/include/optional.svtest | 17 ++++
8 files changed, 126 insertions(+), 50 deletions(-)
create mode 100644 tests/extensions/include/execute/optional-missing.sieve
diff --git a/src/lib-sieve/plugins/include/cmd-include.c b/src/lib-sieve/plugins/include/cmd-include.c
index 4d6f7a7bf..38135a773 100644
--- a/src/lib-sieve/plugins/include/cmd-include.c
+++ b/src/lib-sieve/plugins/include/cmd-include.c
@@ -81,7 +81,7 @@ const struct sieve_operation_def include_operation = {
struct cmd_include_context_data {
enum ext_include_script_location location;
-
+ const char *script_name;
struct sieve_script *script;
enum ext_include_flags flags;
@@ -328,6 +328,7 @@ cmd_include_validate(struct sieve_validator *valdtr,
}
ext_include_ast_link_included_script(cmd->ext, cmd->ast_node->ast, script);
+ ctx_data->script_name = p_strdup(sieve_command_pool(cmd), script_name);
ctx_data->script = script;
(void)sieve_ast_arguments_detach(arg, 1);
@@ -351,6 +352,7 @@ cmd_include_generate(const struct sieve_codegen_env *cgenv,
This yields the id of the binary block containing the compiled byte
code. */
ret = ext_include_generate_include(cgenv, cmd, ctx_data->location,
+ ctx_data->script_name,
ctx_data->flags, ctx_data->script,
&included);
if (ret < 0)
@@ -385,7 +387,7 @@ static bool opc_include_dump(const struct sieve_dumptime_env *denv,
binctx = ext_include_binary_get_context(denv->oprtn->ext, denv->sbin);
included = ext_include_binary_script_get_included(binctx, include_id);
- if (included == NULL)
+ if (included == NULL || included->block == NULL)
return FALSE;
sieve_code_descend(denv);
diff --git a/src/lib-sieve/plugins/include/ext-include-binary.c b/src/lib-sieve/plugins/include/ext-include-binary.c
index ce72b3425..0d5d76128 100644
--- a/src/lib-sieve/plugins/include/ext-include-binary.c
+++ b/src/lib-sieve/plugins/include/ext-include-binary.c
@@ -59,7 +59,7 @@ struct ext_include_binary_context {
struct sieve_binary *binary;
struct sieve_binary_block *dependency_block;
- HASH_TABLE(struct sieve_script *,
+ HASH_TABLE(const struct ext_include_script_info *,
struct ext_include_script_info *) included_scripts;
ARRAY(struct ext_include_script_info *) include_index;
@@ -68,6 +68,32 @@ struct ext_include_binary_context {
bool outdated:1;
};
+static int
+ext_include_script_info_cmp(const struct ext_include_script_info *sinfo1,
+ const struct ext_include_script_info *sinfo2)
+{
+ if (sinfo1 == sinfo2)
+ return 0;
+ if (sinfo1 == NULL || sinfo2 == NULL)
+ return (sinfo1 == NULL ? -1 : 1);
+ if (sinfo1->script == NULL || sinfo2->script == NULL) {
+ if (sinfo1->location != sinfo2->location)
+ return (sinfo1->location > sinfo2->location ? 1 : -1);
+ return strcmp(sinfo1->script_name, sinfo2->script_name);
+ }
+
+ return sieve_script_cmp(sinfo1->script, sinfo2->script);
+}
+
+static unsigned int
+ext_include_script_info_hash(const struct ext_include_script_info *sinfo)
+{
+ if (sinfo == NULL)
+ return 0;
+ return ((unsigned int)sinfo->location +
+ str_hash(sinfo->script_name));
+}
+
static struct ext_include_binary_context *
ext_include_binary_create_context(const struct sieve_extension *this_ext,
struct sieve_binary *sbin)
@@ -79,7 +105,8 @@ ext_include_binary_create_context(const struct sieve_extension *this_ext,
ctx->binary = sbin;
hash_table_create(&ctx->included_scripts, pool, 0,
- sieve_script_hash, sieve_script_cmp);
+ ext_include_script_info_hash,
+ ext_include_script_info_cmp);
p_array_init(&ctx->include_index, pool, 128);
sieve_binary_extension_set(sbin, this_ext, &include_binary_ext, ctx);
@@ -132,24 +159,29 @@ ext_include_binary_init(const struct sieve_extension *this_ext,
struct ext_include_script_info *
ext_include_binary_script_include(struct ext_include_binary_context *binctx,
enum ext_include_script_location location,
+ const char *script_name,
enum ext_include_flags flags,
struct sieve_script *script,
struct sieve_binary_block *inc_block)
{
pool_t pool = sieve_binary_pool(binctx->binary);
struct ext_include_script_info *incscript;
+ const struct ext_include_script_info *key;
incscript = p_new(pool, struct ext_include_script_info, 1);
incscript->id = array_count(&binctx->include_index)+1;
incscript->location = location;
+ incscript->script_name = p_strdup(pool, script_name);
incscript->flags = flags;
incscript->script = script;
incscript->block = inc_block;
+ key = incscript;
/* Unreferenced on binary_free */
- sieve_script_ref(script);
+ if (script != NULL)
+ sieve_script_ref(script);
- hash_table_insert(binctx->included_scripts, script, incscript);
+ hash_table_insert(binctx->included_scripts, key, incscript);
array_append(&binctx->include_index, &incscript, 1);
return incscript;
@@ -157,12 +189,18 @@ ext_include_binary_script_include(struct ext_include_binary_context *binctx,
struct ext_include_script_info *
ext_include_binary_script_get_include_info(
- struct ext_include_binary_context *binctx, struct sieve_script *script)
+ struct ext_include_binary_context *binctx,
+ enum ext_include_script_location location, const char *script_name)
{
- struct ext_include_script_info *incscript =
- hash_table_lookup(binctx->included_scripts, script);
+ struct ext_include_script_info key_def;
+ const struct ext_include_script_info *key;
- return incscript;
+ i_zero(&key_def);
+ key_def.location = location;
+ key_def.script_name = script_name;
+ key = &key_def;
+
+ return hash_table_lookup(binctx->included_scripts, key);
}
const struct ext_include_script_info *
@@ -179,13 +217,6 @@ ext_include_binary_script_get_included(
return NULL;
}
-const struct ext_include_script_info *
-ext_include_binary_script_get(struct ext_include_binary_context *binctx,
- struct sieve_script *script)
-{
- return hash_table_lookup(binctx->included_scripts, script);
-}
-
unsigned int
ext_include_binary_script_get_count(struct ext_include_binary_context *binctx)
{
@@ -239,10 +270,12 @@ ext_include_binary_pre_save(const struct sieve_extension *ext ATTR_UNUSED,
sieve_binary_emit_unsigned(sblock, 0);
}
sieve_binary_emit_byte(sblock, incscript->location);
- sieve_binary_emit_cstring(
- sblock, sieve_script_name(incscript->script));
+ sieve_binary_emit_cstring(sblock, incscript->script_name);
sieve_binary_emit_byte(sblock, incscript->flags);
- sieve_script_binary_write_metadata(incscript->script, sblock);
+ if (incscript->block != NULL) {
+ sieve_script_binary_write_metadata(incscript->script,
+ sblock);
+ }
}
result = ext_include_variables_save(sblock, binctx->global_vars,
@@ -386,8 +419,9 @@ ext_include_binary_open(const struct sieve_extension *ext,
}
/* Can we read script metadata ? */
- ret = sieve_script_binary_read_metadata(script, sblock,
- &offset);
+ ret = (inc_block == NULL ? 1 :
+ sieve_script_binary_read_metadata(script, sblock,
+ &offset));
if (ret < 0) {
/* Binary is corrupt, recompile */
e_error(svinst->event, "include: "
@@ -402,8 +436,10 @@ ext_include_binary_open(const struct sieve_extension *ext,
if (ret == 0)
binctx->outdated = TRUE;
- (void)ext_include_binary_script_include(binctx, location, flags,
- script, inc_block);
+ (void)ext_include_binary_script_include(binctx, location,
+ str_c(script_name),
+ flags, script,
+ inc_block);
sieve_script_unref(&script);
}
@@ -432,13 +468,13 @@ ext_include_binary_free(const struct sieve_extension *ext ATTR_UNUSED,
struct ext_include_binary_context *binctx =
(struct ext_include_binary_context *)context;
struct hash_iterate_context *hctx;
- struct sieve_script *script;
+ const struct ext_include_script_info *key;
struct ext_include_script_info *incscript;
/* Release references to all included script objects */
hctx = hash_table_iterate_init(binctx->included_scripts);
while (hash_table_iterate(hctx, binctx->included_scripts,
- &script, &incscript))
+ &key, &incscript))
sieve_script_unref(&incscript->script);
hash_table_iterate_deinit(&hctx);
@@ -459,7 +495,7 @@ bool ext_include_binary_dump(const struct sieve_extension *ext,
struct ext_include_binary_context *binctx =
ext_include_binary_get_context(ext, sbin);
struct hash_iterate_context *hctx;
- struct sieve_script *script;
+ const struct ext_include_script_info *key;
struct ext_include_script_info *incscript;
if (!ext_include_variables_dump(denv, binctx->global_vars))
@@ -467,13 +503,13 @@ bool ext_include_binary_dump(const struct sieve_extension *ext,
hctx = hash_table_iterate_init(binctx->included_scripts);
while (hash_table_iterate(hctx, binctx->included_scripts,
- &script, &incscript)) {
+ &key, &incscript)) {
if (incscript->block == NULL) {
sieve_binary_dump_sectionf(
denv, "Included %s script '%s' (MISSING)",
ext_include_script_location_name(
incscript->location),
- sieve_script_name(incscript->script));
+ incscript->script_name);
} else {
unsigned int block_id =
sieve_binary_block_get_id(incscript->block);
@@ -482,7 +518,7 @@ bool ext_include_binary_dump(const struct sieve_extension *ext,
denv, "Included %s script '%s' (block: %d)",
ext_include_script_location_name(
incscript->location),
- sieve_script_name(incscript->script), block_id);
+ incscript->script_name, block_id);
denv->sblock = incscript->block;
denv->cdumper = sieve_code_dumper_create(denv);
diff --git a/src/lib-sieve/plugins/include/ext-include-binary.h b/src/lib-sieve/plugins/include/ext-include-binary.h
index d3c6e62c6..cfc09c88d 100644
--- a/src/lib-sieve/plugins/include/ext-include-binary.h
+++ b/src/lib-sieve/plugins/include/ext-include-binary.h
@@ -31,9 +31,11 @@ ext_include_binary_get_global_scope(const struct sieve_extension *this_ext,
struct ext_include_script_info {
unsigned int id;
+ enum ext_include_script_location location;
+ const char *script_name;
+
struct sieve_script *script;
enum ext_include_flags flags;
- enum ext_include_script_location location;
struct sieve_binary_block *block;
};
@@ -41,19 +43,18 @@ struct ext_include_script_info {
struct ext_include_script_info *
ext_include_binary_script_include(struct ext_include_binary_context *binctx,
enum ext_include_script_location location,
+ const char *script_name,
enum ext_include_flags flags,
struct sieve_script *script,
struct sieve_binary_block *inc_block);
struct ext_include_script_info *
ext_include_binary_script_get_include_info(
- struct ext_include_binary_context *binctx, struct sieve_script *script);
+ struct ext_include_binary_context *binctx,
+ enum ext_include_script_location location, const char *script_name);
const struct ext_include_script_info *
ext_include_binary_script_get_included(
struct ext_include_binary_context *binctx, unsigned int include_id);
-const struct ext_include_script_info *
-ext_include_binary_script_get(struct ext_include_binary_context *binctx,
- struct sieve_script *script);
unsigned int
ext_include_binary_script_get_count(struct ext_include_binary_context *binctx);
diff --git a/src/lib-sieve/plugins/include/ext-include-common.c b/src/lib-sieve/plugins/include/ext-include-common.c
index a8972dfd3..04bc4b7f3 100644
--- a/src/lib-sieve/plugins/include/ext-include-common.c
+++ b/src/lib-sieve/plugins/include/ext-include-common.c
@@ -32,6 +32,8 @@
struct ext_include_generator_context {
unsigned int nesting_depth;
+ enum ext_include_script_location location;
+ const char *script_name;
struct sieve_script *script;
struct ext_include_generator_context *parent;
};
@@ -271,6 +273,7 @@ static struct ext_include_generator_context *
ext_include_create_generator_context(
struct sieve_generator *gentr,
struct ext_include_generator_context *parent,
+ enum ext_include_script_location location, const char *script_name,
struct sieve_script *script)
{
struct ext_include_generator_context *ctx;
@@ -278,6 +281,8 @@ ext_include_create_generator_context(
pool_t pool = sieve_generator_pool(gentr);
ctx = p_new(pool, struct ext_include_generator_context, 1);
ctx->parent = parent;
+ ctx->location = location;
+ ctx->script_name = p_strdup(pool, script_name);
ctx->script = script;
if (parent == NULL)
ctx->nesting_depth = 0;
@@ -299,11 +304,14 @@ static inline void
ext_include_initialize_generator_context(
const struct sieve_extension *this_ext, struct sieve_generator *gentr,
struct ext_include_generator_context *parent,
+ enum ext_include_script_location location, const char *script_name,
struct sieve_script *script)
{
sieve_generator_extension_set_context(
gentr, this_ext,
- ext_include_create_generator_context(gentr, parent, script));
+ ext_include_create_generator_context(gentr, parent,
+ location, script_name,
+ script));
}
void ext_include_register_generator_context(
@@ -315,8 +323,10 @@ void ext_include_register_generator_context(
/* Initialize generator context if necessary */
if (ctx == NULL) {
+ i_assert(cgenv->script != NULL);
ctx = ext_include_create_generator_context(
- cgenv->gentr, NULL, cgenv->script);
+ cgenv->gentr, NULL, EXT_INCLUDE_LOCATION_PERSONAL,
+ sieve_script_name(cgenv->script), cgenv->script);
sieve_generator_extension_set_context(
cgenv->gentr, this_ext, (void *)ctx);
@@ -458,8 +468,8 @@ ext_include_interpreter_get_global_variables(
int ext_include_generate_include(
const struct sieve_codegen_env *cgenv, struct sieve_command *cmd,
- enum ext_include_script_location location, enum ext_include_flags flags,
- struct sieve_script *script,
+ enum ext_include_script_location location, const char *script_name,
+ enum ext_include_flags flags, struct sieve_script *script,
const struct ext_include_script_info **included_r)
{
const struct sieve_extension *this_ext = cmd->ext;
@@ -499,7 +509,10 @@ int ext_include_generate_include(
if ((flags & EXT_INCLUDE_FLAG_ONCE) == 0) {
pctx = ctx;
while (pctx != NULL) {
- if (sieve_script_equals(pctx->script, script)) {
+ if (pctx->location == location &&
+ strcmp(pctx->script_name, script_name) == 0 &&
+ (pctx->script == NULL || script == NULL ||
+ sieve_script_equals(pctx->script, script))) {
/* Just drop circular include when uploading
inactive script; not an error
*/
@@ -524,7 +537,8 @@ int ext_include_generate_include(
binctx = ext_include_binary_init(this_ext, sbin, cgenv->ast);
/* Is the script already compiled into the current binary? */
- included = ext_include_binary_script_get_include_info(binctx, script);
+ included = ext_include_binary_script_get_include_info(
+ binctx, location, script_name);
if (included != NULL) {
/* Yes, only update flags */
if ((flags & EXT_INCLUDE_FLAG_OPTIONAL) == 0)
@@ -532,7 +546,6 @@ int ext_include_generate_include(
if ((flags & EXT_INCLUDE_FLAG_ONCE) == 0)
included->flags &= ENUM_NEGATE(EXT_INCLUDE_FLAG_ONCE);
} else {
- const char *script_name = sieve_script_name(script);
enum sieve_compile_flags cpflags = cgenv->flags;
/* No, include new script */
@@ -550,13 +563,14 @@ int ext_include_generate_include(
/* Allocate a new block in the binary and mark the script as
included. */
- if (!sieve_script_is_open(script)) {
+ if (script == NULL || !sieve_script_is_open(script)) {
/* Just making an empty entry to mark a missing script
*/
i_assert((flags & EXT_INCLUDE_FLAG_MISSING_AT_UPLOAD) != 0 ||
(flags & EXT_INCLUDE_FLAG_OPTIONAL) != 0);
included = ext_include_binary_script_include(
- binctx, location, flags, script, NULL);
+ binctx, location, script_name, flags, NULL,
+ NULL);
result = 0;
} else {
struct sieve_binary_block *inc_block =
@@ -564,7 +578,8 @@ int ext_include_generate_include(
/* Real include */
included = ext_include_binary_script_include(
- binctx, location, flags, script, inc_block);
+ binctx, location, script_name, flags, script,
+ inc_block);
/* Parse */
if ((ast = sieve_parse(script, ehandler,
@@ -604,7 +619,8 @@ int ext_include_generate_include(
*/
subgentr = sieve_generator_create(ast, ehandler, cpflags);
ext_include_initialize_generator_context(
- cmd->ext, subgentr, ctx, script);
+ cmd->ext, subgentr, ctx, location, script_name,
+ script);
if (sieve_generator_run(subgentr, &inc_block) == NULL) {
sieve_command_generate_error(
@@ -683,7 +699,7 @@ int ext_include_execute_include(const struct sieve_runtime_env *renv,
/* Check for invalid include id (== corrupt binary) */
included = ext_include_binary_script_get_included(binctx, include_id);
- if (included == NULL) {
+ if (included == NULL || included->block == NULL) {
sieve_runtime_trace_error(
renv, "include: include id %d is invalid", include_id);
return SIEVE_EXEC_BIN_CORRUPT;
diff --git a/src/lib-sieve/plugins/include/ext-include-common.h b/src/lib-sieve/plugins/include/ext-include-common.h
index ca586b7e6..b1a793f15 100644
--- a/src/lib-sieve/plugins/include/ext-include-common.h
+++ b/src/lib-sieve/plugins/include/ext-include-common.h
@@ -143,8 +143,8 @@ void ext_include_register_generator_context(
int ext_include_generate_include(
const struct sieve_codegen_env *cgenv, struct sieve_command *cmd,
- enum ext_include_script_location location, enum ext_include_flags flags,
- struct sieve_script *script,
+ enum ext_include_script_location location, const char *script_name,
+ enum ext_include_flags flags, struct sieve_script *script,
const struct ext_include_script_info **included_r);
/* Interpreter context */
diff --git a/src/lib-sieve/plugins/include/ext-include.c b/src/lib-sieve/plugins/include/ext-include.c
index f85bd2489..a8423beb5 100644
--- a/src/lib-sieve/plugins/include/ext-include.c
+++ b/src/lib-sieve/plugins/include/ext-include.c
@@ -67,7 +67,7 @@ ext_include_binary_load(const struct sieve_extension *ext,
const struct sieve_extension_def include_extension = {
.name = "include",
- .version = 1,
+ .version = 2,
.load = ext_include_load,
.unload = ext_include_unload,
diff --git a/tests/extensions/include/execute/optional-missing.sieve b/tests/extensions/include/execute/optional-missing.sieve
new file mode 100644
index 000000000..e80569a8d
--- /dev/null
+++ b/tests/extensions/include/execute/optional-missing.sieve
@@ -0,0 +1,4 @@
+require "include";
+
+include :personal :optional "missing_a";
+include :personal :optional "missing_aa";
diff --git a/tests/extensions/include/optional.svtest b/tests/extensions/include/optional.svtest
index 345f8309b..0b19c9d9d 100644
--- a/tests/extensions/include/optional.svtest
+++ b/tests/extensions/include/optional.svtest
@@ -38,3 +38,20 @@ test "Included Optional - Binary" {
test_fail "unexpected result value: ${result}";
}
}
+
+test "Included Optional - Binary Missing" {
+ if not test_script_compile "execute/optional-missing.sieve" {
+ test_fail "failed to compile sieve script";
+ }
+
+ test_binary_save "optional-missing";
+ test_binary_load "optional-missing";
+
+ if not test_script_run {
+ test_fail "failed to execute sieve script";
+ }
+
+ if not string "${result}" " ONE TWO" {
+ test_fail "unexpected result value: ${result}";
+ }
+}
--
GitLab