sieve crashes when two optional include scripts are missing

4 Jun 2024


      Hi,
we've found an issue with sieve plugin. When script includes two missing
optional scripts, it causes crash.
For reproducing the issue, script names must belong to same hash table
node, so either two scripts with same name or scripts where 'hash(name)
% table->size' results in same value, like missing_a and missing_aa in
reproducer below.
Reduced reproducer:
cat >dovecot.min.conf <<EOF
mail_location = maildir:~/Maildir
plugin {
sieve = file:/tmp;active=/tmp/dovecot.sieve
}
EOF
cat >reproducer.sieve <<EOF
require "include";
include :personal :optional "missing_a";
include :personal :optional "missing_aa";
EOF
echo -n > empty
sieve-test -c dovecot.min.conf reproducer.sieve empty
any test mail would do, but empty file is enough for reproducer
# return sblock->id causes the crash as sblock is NULL
backtrace:
(gdb) bt
#0  sieve_binary_block_get_id (sblock=0x0) at
src/lib-sieve/sieve-binary.c:421
#1  ext_include_execute_include (renv=0x5555555bbed8, include_id=1,
flags=EXT_INCLUDE_FLAG_OPTIONAL) at plugins/include/ext-include-common.c:696
#2  opc_include_execute (renv=0x5555555bbed8, address=0x5555555bbf10) at
plugins/include/cmd-include.c:399
#3  sieve_interpreter_operation_execute (interp=0x5555555bbe88)
at src/lib-sieve/sieve-interpreter.c:901
#4  sieve_interpreter_continue (interp=0x5555555bbe88, interrupted=0x0)
at src/lib-sieve/sieve-interpreter.c:959
#5  sieve_interpreter_start (interp=0x5555555bbe88,
result=0x5555555b9230, interrupted=0x0)
at src/lib-sieve/sieve-interpreter.c:1049
#6  sieve_interpreter_run (interp=0x5555555bbe88, result=0x5555555b9230)
at src/lib-sieve/sieve-interpreter.c:1057
#7  sieve_run (sbin=0x55555558ee18, result=0x5555555b9230,
eenv=0x7fffffffe890, ehandler=0x55555558c338)
at src/lib-sieve/sieve.c:357
#8  sieve_test (sbin=0x55555558ee18, msgdata=0x7fffffffe9f0,
senv=0x7fffffffea30, ehandler=0x55555558c338, stream=0x5555555b2cd0,
flags=SIEVE_EXECUTE_FLAG_LOG_RESULT) at src/lib-sieve/sieve.c:598
#9  main (argc=5, argv=0x555555567a10) at src/sieve-tools/sieve-test.c:391
issue seems to be caused by
lib-sieve/plugins/include/ext-include-common.c: ext_include_generate_include
on first pass (script) the check for script already compiled into binary at
529: included = ext_include_binary_script_get_include_info(binctx, script);
returns NULL, so code goes through 'else' block where it hits
sieve_script_is_open(script) check. It includes script with NULL block,
returns 0 so caller cmd_include_generate does not emit anything. On
second pass the above 'included' check returns first missing script, it
goes through true block where it just checks flags and returns non-zero,
so caller this time goes through the emit code block and the previously
included NULL sblock gets referenced causing crash later.
Note: despite scripts have different name, they pass the "same script"
check because sieve_file_script_equals compares 0==0 dev & inode numbers
as scripts do not exist. This is why scripts do not have to have same
names. Side effect of this is that as missing_a and missing_aa are
treated like the same script, only first one is linked in compiled
svbin. Which means that if missing_aa reappears later, svbin won't get
recompiled.
Another manifestation of the issue is when included missing_a script is
present when main script is compiled and later missina_a is removed
again. During execution, script is not recompiled and it aborts for hash
table dupe check:
sieve_binary_open->ext_include_binary_script_include->hash_table_insert
There are several ways how to solve this. I've tried a few, but there
were other corner cases that appeared. One tried way was for example
ext_include_generate_include in the 'if (included)' replicate the check
sieve_script_is_open(...) from else block, just check flags, no include
and set result to 0. Another way to solve this is to return from
ext_include_execute_include early if sblock is NULL just before it tries
to derefernce it, similarly as it does if it fails 'once' check. Same
for dumping in opc_include_dump. Whilst the first approach seems a bit
better, it won't stop crashing in existing environments as it won't
trigger svbin recompilation and svbin would fail. This means that the
second soultion is sort of needed anyway. This still won't solve the
problem with different missing scripts as only first one would be able
to trigger recompilation when file becomes available. Just including
both in the mentioned function won't work either as they would still be
evaluated as same and hash table anti-dupe check would trigger abort. So
some changes in script comparison seem needed anyway. This is what I've
used in my approach. Return early from ext_include_execute_include and
opc_include_dump if sblock is NULL and change sieve_file_script_equals
to compare also script->location for missing scripts (when inode is 0).
Let me know if you need more information.
Cheers,
Michal Hlavinka

Michal Hlavinka

Stephan Bosch

tags

participants (2)