diff --git a/CHANGES b/CHANGES index fd2b097411..8fc720d1d6 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,17 @@ +5.2.0-dev.171 | 2022-11-01 07:47:41 -0700 + + * Func: Do not crash on va_args confusion for script funcs (Arne Welzel, Corelight) + + Script and BIF functions with a single any parameter are excluded from + type checking regarding arguments. This makes it possible to call a + ScriptFunc with more arguments than it actually has parameters and frame + space for, causing heap-buffer-overflows. + + This change runtime checks expected parameters and provided arguments + and short-circuits execution as well as logging runtime expression errors. + + Fixes #2446 + 5.2.0-dev.169 | 2022-10-31 15:18:04 -0700 * bifs/record_fields: Include actual enum name in type_name (Arne Welzel, Corelight) diff --git a/VERSION b/VERSION index 38e701688b..d205acf587 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -5.2.0-dev.169 +5.2.0-dev.171 diff --git a/src/Func.cc b/src/Func.cc index 4b5f4e7e8e..4d2abf9f41 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -362,6 +362,17 @@ ValPtr ScriptFunc::Invoke(zeek::Args* args, Frame* parent) const const CallExpr* call_expr = parent ? parent->GetCall() : nullptr; call_stack.emplace_back(CallInfo{call_expr, this, *args}); + // If a script function is ever invoked with more arguments than it has + // parameters log an error and return. Most likely a "variadic function" + // that only has a single any parameter and is excluded from static type + // checking is involved. This should otherwise not be possible to hit. + auto num_params = static_cast(GetType()->Params()->NumFields()); + if ( args->size() > num_params ) + { + emit_builtin_exception("too many arguments for function call"); + return nullptr; + } + if ( etm && Flavor() == FUNC_FLAVOR_EVENT ) etm->StartEvent(this, args); diff --git a/testing/btest/Baseline/language.any-script-func-variadic-errors-2/.stderr b/testing/btest/Baseline/language.any-script-func-variadic-errors-2/.stderr new file mode 100644 index 0000000000..5fe801eb80 --- /dev/null +++ b/testing/btest/Baseline/language.any-script-func-variadic-errors-2/.stderr @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +expression error in <...>/any-script-func-variadic-errors.zeek, line 14: too many arguments for function call (f(1, 2)) diff --git a/testing/btest/Baseline/language.any-script-func-variadic-errors-2/output b/testing/btest/Baseline/language.any-script-func-variadic-errors-2/output new file mode 100644 index 0000000000..8f43ac4e92 --- /dev/null +++ b/testing/btest/Baseline/language.any-script-func-variadic-errors-2/output @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +l=lambda local x=1 diff --git a/testing/btest/Baseline/language.any-script-func-variadic-errors/.stderr b/testing/btest/Baseline/language.any-script-func-variadic-errors/.stderr new file mode 100644 index 0000000000..dba98c4350 --- /dev/null +++ b/testing/btest/Baseline/language.any-script-func-variadic-errors/.stderr @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +expression error in <...>/any-script-func-variadic-errors.zeek, line 15: too many arguments for function call (f(1, 2)) diff --git a/testing/btest/Baseline/language.any-script-func-variadic-errors/output b/testing/btest/Baseline/language.any-script-func-variadic-errors/output new file mode 100644 index 0000000000..a6822601c7 --- /dev/null +++ b/testing/btest/Baseline/language.any-script-func-variadic-errors/output @@ -0,0 +1,6 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +zeek_init() &priority=10 +l=a local x=1 +zeek_init() &priority=-10 +l=a local x=1 +l=a local x=1 diff --git a/testing/btest/language/any-script-func-variadic-errors.zeek b/testing/btest/language/any-script-func-variadic-errors.zeek new file mode 100644 index 0000000000..6d1f78ebcb --- /dev/null +++ b/testing/btest/language/any-script-func-variadic-errors.zeek @@ -0,0 +1,47 @@ +# @TEST-EXEC: zeek -b %INPUT >output +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr + +function f(x: any) + { + local l = "a local"; + print fmt("l=%s x=%s", l, x); + } + +event zeek_init() &priority=10 + { + print "zeek_init() &priority=10"; + f(1); + f(1, 2); + # Not reached + print "FAIL"; + f(1); + } + +event zeek_init() &priority=-10 + { + print "zeek_init() &priority=-10"; + f(1); + f(1); + } + + +@TEST-START-NEXT +# Do not allow to call variadic through a script-level variable. +global f: function(x: any); + +event zeek_init() + { + local _lambda = function(x: any) { + local l = "lambda local"; + print fmt("l=%s x=%s", l, x); + }; + + f = _lambda; + + f(1); + f(1, 2); + # Not reached + print "FAIL"; + f(1); + }