Merge remote-tracking branch 'origin/topic/awelzel/2446-runtime-error-for-variadic-functions'

* origin/topic/awelzel/2446-runtime-error-for-variadic-functions:
  Func: Do not crash on va_args confusion for script funcs

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)

VERSION

@@ -1 +1 @@
-5.2.0-dev.169
+5.2.0-dev.171

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<size_t>(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);
 

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))

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

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))

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

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);
+	}