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 < cat >reproducer.sieve < 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