From 4314467e448369da8f33451b5551efa3482106ad Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 28 Oct 2022 13:55:57 +0200 Subject: [PATCH] Func: Do not crash on va_args confusion for script funcs 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 --- src/Func.cc | 11 +++++ .../.stderr | 2 + .../output | 2 + .../.stderr | 2 + .../output | 6 +++ .../any-script-func-variadic-errors.zeek | 47 +++++++++++++++++++ 6 files changed, 70 insertions(+) create mode 100644 testing/btest/Baseline/language.any-script-func-variadic-errors-2/.stderr create mode 100644 testing/btest/Baseline/language.any-script-func-variadic-errors-2/output create mode 100644 testing/btest/Baseline/language.any-script-func-variadic-errors/.stderr create mode 100644 testing/btest/Baseline/language.any-script-func-variadic-errors/output create mode 100644 testing/btest/language/any-script-func-variadic-errors.zeek 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); + }