From 9f16fa6474e479493dad86a947a47f6b4a9106e7 Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@corelight.com>
Date: Wed, 24 Jun 2020 20:21:12 -0700
Subject: [PATCH 1/3] GH-1024: fix crash on passing wrong types to Broker store
 API

---
 src/broker/store.bif                          | 97 +++++++++++--------
 .../Baseline/broker.store.invalid-handle/out  |  2 +
 .../btest/broker/store/invalid-handle.zeek    | 21 ++++
 3 files changed, 81 insertions(+), 39 deletions(-)
 create mode 100644 testing/btest/Baseline/broker.store.invalid-handle/out
 create mode 100644 testing/btest/broker/store/invalid-handle.zeek

diff --git a/src/broker/store.bif b/src/broker/store.bif
index 68262f667b..340936ebda 100644
--- a/src/broker/store.bif
+++ b/src/broker/store.bif
@@ -20,6 +20,9 @@ static broker::optional<broker::timespan> prepare_expiry(double e)
 
 	return ts;
 	}
+
+static bro_broker::StoreHandleVal* to_store_handle(Val* h)
+	{ return dynamic_cast<bro_broker::StoreHandleVal*>(h); }
 %%}
 
 module Broker;
@@ -87,53 +90,55 @@ function Broker::__create_clone%(id: string, resync_interval: interval,
 function Broker::__is_closed%(h: opaque of Broker::Store%): bool
 	%{
 	bro_broker::Manager::ScriptScopeGuard ssg;
+	auto handle = to_store_handle(h);
 
-	if ( ! h )
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	return val_mgr->Bool(broker_mgr->LookupStore(handle->store.name()));
 	%}
 
 function Broker::__close%(h: opaque of Broker::Store%): bool
 	%{
 	bro_broker::Manager::ScriptScopeGuard ssg;
+	auto handle = to_store_handle(h);
 
-	if ( ! h )
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	return val_mgr->Bool(broker_mgr->CloseStore(handle->store.name()));
 	%}
 
 function Broker::__store_name%(h: opaque of Broker::Store%): string
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->EmptyString();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	return make_intrusive<StringVal>(handle->store.name());
 	%}
 
 function Broker::__exists%(h: opaque of Broker::Store,
                            k: any%): Broker::QueryResult
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
@@ -172,13 +177,14 @@ function Broker::__exists%(h: opaque of Broker::Store,
 function Broker::__get%(h: opaque of Broker::Store,
                         k: any%): Broker::QueryResult
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
@@ -217,13 +223,14 @@ function Broker::__get%(h: opaque of Broker::Store,
 function Broker::__put_unique%(h: opaque of Broker::Store,
                                k: any, v: any, e: interval%): Broker::QueryResult
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto val = bro_broker::val_to_data(v);
 
@@ -271,13 +278,14 @@ function Broker::__put_unique%(h: opaque of Broker::Store,
 function Broker::__get_index_from_value%(h: opaque of Broker::Store,
                                          k: any, i: any%): Broker::QueryResult
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
@@ -324,14 +332,14 @@ function Broker::__get_index_from_value%(h: opaque of Broker::Store,
 
 function Broker::__keys%(h: opaque of Broker::Store%): Broker::QueryResult
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
-
 	auto trigger = frame->GetTrigger();
 
 	if ( ! trigger )
@@ -362,13 +370,14 @@ function Broker::__keys%(h: opaque of Broker::Store%): Broker::QueryResult
 function Broker::__put%(h: opaque of Broker::Store,
                         k: any, v: any, e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto val = bro_broker::val_to_data(v);
 
@@ -390,13 +399,14 @@ function Broker::__put%(h: opaque of Broker::Store,
 
 function Broker::__erase%(h: opaque of Broker::Store, k: any%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
@@ -412,13 +422,14 @@ function Broker::__erase%(h: opaque of Broker::Store, k: any%): bool
 function Broker::__increment%(h: opaque of Broker::Store, k: any, a: any,
                               e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto amount = bro_broker::val_to_data(a);
 
@@ -442,13 +453,14 @@ function Broker::__increment%(h: opaque of Broker::Store, k: any, a: any,
 function Broker::__decrement%(h: opaque of Broker::Store, k: any, a: any,
                               e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto amount = bro_broker::val_to_data(a);
 
@@ -471,13 +483,14 @@ function Broker::__decrement%(h: opaque of Broker::Store, k: any, a: any,
 function Broker::__append%(h: opaque of Broker::Store, k: any, s: any,
                            e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto str = bro_broker::val_to_data(s);
 
@@ -500,13 +513,14 @@ function Broker::__append%(h: opaque of Broker::Store, k: any, s: any,
 function Broker::__insert_into_set%(h: opaque of Broker::Store, k: any, i: any,
                                     e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto idx = bro_broker::val_to_data(i);
 
@@ -530,13 +544,14 @@ function Broker::__insert_into_set%(h: opaque of Broker::Store, k: any, i: any,
 function Broker::__insert_into_table%(h: opaque of Broker::Store, k: any,
                                       i: any, v: any, e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto idx = bro_broker::val_to_data(i);
 	auto val = bro_broker::val_to_data(v);
@@ -567,13 +582,14 @@ function Broker::__insert_into_table%(h: opaque of Broker::Store, k: any,
 function Broker::__remove_from%(h: opaque of Broker::Store, k: any, i: any,
                                 e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto idx = bro_broker::val_to_data(i);
 
@@ -597,13 +613,14 @@ function Broker::__remove_from%(h: opaque of Broker::Store, k: any, i: any,
 function Broker::__push%(h: opaque of Broker::Store, k: any, v: any,
                          e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 	auto val = bro_broker::val_to_data(v);
 
@@ -625,13 +642,14 @@ function Broker::__push%(h: opaque of Broker::Store, k: any, v: any,
 
 function Broker::__pop%(h: opaque of Broker::Store, k: any, e: interval%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
@@ -646,13 +664,14 @@ function Broker::__pop%(h: opaque of Broker::Store, k: any, e: interval%): bool
 
 function Broker::__clear%(h: opaque of Broker::Store%): bool
 	%{
-	if ( ! h )
+	auto handle = to_store_handle(h);
+
+	if ( ! handle )
 		{
 		builtin_error("invalid Broker store handle");
 		return val_mgr->False();
 		}
 
-	auto handle = static_cast<bro_broker::StoreHandleVal*>(h);
 
 	handle->store.clear();
 	return val_mgr->True();
diff --git a/testing/btest/Baseline/broker.store.invalid-handle/out b/testing/btest/Baseline/broker.store.invalid-handle/out
new file mode 100644
index 0000000000..469583e41c
--- /dev/null
+++ b/testing/btest/Baseline/broker.store.invalid-handle/out
@@ -0,0 +1,2 @@
+error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/broker.store.invalid-handle/invalid-handle.zeek, line 6: invalid Broker store handle (Broker::keys(a))
+keys, F
diff --git a/testing/btest/broker/store/invalid-handle.zeek b/testing/btest/broker/store/invalid-handle.zeek
new file mode 100644
index 0000000000..c7e30addbe
--- /dev/null
+++ b/testing/btest/broker/store/invalid-handle.zeek
@@ -0,0 +1,21 @@
+# @TEST-EXEC: zeek -b %INPUT >out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+function print_keys(a: any)
+	{
+	when ( local s = Broker::keys(a) )
+		{
+		print "keys", s;
+		}
+	timeout 2sec
+		{
+		print fmt("<timeout for print keys>");
+		}
+	}
+
+global a: int = 0;
+
+event zeek_init()
+	{
+	print_keys(a);
+	}

From 2b698c4e1b607dfa3a4a5742e2183f3b1221dd9e Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@corelight.com>
Date: Wed, 24 Jun 2020 22:54:46 -0700
Subject: [PATCH 2/3] Add builtin_exception() functions

These work like builtin_error(), but also throw an InterpreterException
---
 src/Func.cc | 67 ++++++++++++++++++++++++++++++++++++++++++-----------
 src/Func.h  |  5 +++-
 2 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/src/Func.cc b/src/Func.cc
index 8bdcf2d2c5..5acb37c8ad 100644
--- a/src/Func.cc
+++ b/src/Func.cc
@@ -663,29 +663,50 @@ void BuiltinFunc::Describe(ODesc* d) const
 	d->AddCount(is_pure);
 	}
 
-void builtin_error(const char* msg)
-	{
-	builtin_error(msg, IntrusivePtr<Val>{});
-	}
-
-void builtin_error(const char* msg, IntrusivePtr<Val> arg)
-	{
-	builtin_error(msg, arg.get());
-	}
-
-void builtin_error(const char* msg, BroObj* arg)
+static void builtin_error_common(const char* msg, BroObj* arg, bool unwind)
 	{
 	auto emit = [=](const zeek::detail::CallExpr* ce)
 		{
 		if ( ce )
-			ce->Error(msg, arg);
+			{
+			if ( unwind )
+				{
+				if ( arg )
+					{
+					ODesc d;
+					arg->Describe(&d);
+					reporter->ExprRuntimeError(ce, "%s (%s), during call:", msg,
+					                           d.Description());
+					}
+				else
+					reporter->ExprRuntimeError(ce, "%s", msg);
+				}
+			else
+				ce->Error(msg, arg);
+			}
 		else
-			reporter->Error(msg, arg);
+			{
+			if ( arg )
+				{
+				if ( unwind )
+					reporter->RuntimeError(arg->GetLocationInfo(), "%s", msg);
+				else
+					arg->Error(msg);
+				}
+			else
+				{
+				if ( unwind )
+					reporter->RuntimeError(nullptr, "%s", msg);
+				else
+					reporter->Error("%s", msg);
+				}
+			}
 		};
 
-
 	if ( call_stack.empty() )
 		{
+		// Shouldn't happen unless someone (mistakenly) calls builtin_error()
+		// from somewhere that's not even evaluating script-code.
 		emit(nullptr);
 		return;
 		}
@@ -739,6 +760,24 @@ void builtin_error(const char* msg, BroObj* arg)
 		emit(last_call.call);
 	}
 
+void builtin_error(const char* msg)
+	{ builtin_error_common(msg, nullptr, false); }
+
+void builtin_error(const char* msg, const IntrusivePtr<Val>& arg)
+	{ builtin_error_common(msg, arg.get(), false); }
+
+void builtin_error(const char* msg, BroObj* arg)
+	{ builtin_error_common(msg, arg, false); }
+
+void builtin_exception(const char* msg)
+	{ builtin_error_common(msg, nullptr, true); }
+
+void builtin_exception(const char* msg, const IntrusivePtr<Val>& arg)
+	{ builtin_error_common(msg, arg.get(), true); }
+
+void builtin_exception(const char* msg, BroObj* arg)
+	{ builtin_error_common(msg, arg, true); }
+
 #include "zeek.bif.func_h"
 #include "stats.bif.func_h"
 #include "reporter.bif.func_h"
diff --git a/src/Func.h b/src/Func.h
index b1a2eae796..fe391a3e8a 100644
--- a/src/Func.h
+++ b/src/Func.h
@@ -236,8 +236,11 @@ protected:
 
 
 extern void builtin_error(const char* msg);
-extern void builtin_error(const char* msg, IntrusivePtr<Val>);
+extern void builtin_error(const char* msg, const IntrusivePtr<Val>& arg);
 extern void builtin_error(const char* msg, BroObj* arg);
+extern void builtin_exception(const char* msg);
+extern void builtin_exception(const char* msg, const IntrusivePtr<Val>& arg);
+extern void builtin_exception(const char* msg, BroObj* arg);
 extern void init_builtin_funcs();
 extern void init_builtin_funcs_subdirs();
 

From a9f853efcd150644be3660c7763a498b59eb0bc0 Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@corelight.com>
Date: Wed, 24 Jun 2020 22:56:14 -0700
Subject: [PATCH 3/3] Improve Broker store API's handling of invalid arguments

* Some methods mistakenly returned a bool instead of QueryResult
  when passed an invalid `opaque of Broker::Store` handle.

* Now generates a runtime exception for store_name() and is_closed()
  calls that pass an invalid `opaque of Broker::Store` handle as any
  returned value can't be reasonably used in any subsequent logic.

* Descriptions of any invalid arguments are now given in the error
  message.
---
 src/broker/store.bif                          | 107 ++++++++----------
 .../Baseline/broker.store.invalid-handle/out  |   5 +-
 .../btest/broker/store/invalid-handle.zeek    |  13 +++
 3 files changed, 66 insertions(+), 59 deletions(-)

diff --git a/src/broker/store.bif b/src/broker/store.bif
index 340936ebda..bce3da5e75 100644
--- a/src/broker/store.bif
+++ b/src/broker/store.bif
@@ -93,10 +93,7 @@ function Broker::__is_closed%(h: opaque of Broker::Store%): bool
 	auto handle = to_store_handle(h);
 
 	if ( ! handle )
-		{
-		builtin_error("invalid Broker store handle");
-		return val_mgr->False();
-		}
+		builtin_exception("invalid Broker store handle", h);
 
 	return val_mgr->Bool(broker_mgr->LookupStore(handle->store.name()));
 	%}
@@ -108,7 +105,7 @@ function Broker::__close%(h: opaque of Broker::Store%): bool
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -120,10 +117,7 @@ function Broker::__store_name%(h: opaque of Broker::Store%): string
 	auto handle = to_store_handle(h);
 
 	if ( ! handle )
-		{
-		builtin_error("invalid Broker store handle");
-		return val_mgr->EmptyString();
-		}
+		builtin_exception("invalid Broker store handle", h);
 
 	return make_intrusive<StringVal>(handle->store.name());
 	%}
@@ -135,15 +129,15 @@ function Broker::__exists%(h: opaque of Broker::Store,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
-		return val_mgr->False();
+		builtin_error("invalid Broker store handle", h);
+		return bro_broker::query_result();
 		}
 
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return bro_broker::query_result();
 		}
 
@@ -181,15 +175,15 @@ function Broker::__get%(h: opaque of Broker::Store,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
-		return val_mgr->False();
+		builtin_error("invalid Broker store handle", h);
+		return bro_broker::query_result();
 		}
 
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return bro_broker::query_result();
 		}
 
@@ -227,8 +221,8 @@ function Broker::__put_unique%(h: opaque of Broker::Store,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
-		return val_mgr->False();
+		builtin_error("invalid Broker store handle", h);
+		return bro_broker::query_result();
 		}
 
 	auto key = bro_broker::val_to_data(k);
@@ -236,13 +230,13 @@ function Broker::__put_unique%(h: opaque of Broker::Store,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return bro_broker::query_result();
 		}
 
 	if ( ! val )
 		{
-		builtin_error("invalid Broker data conversion for value argument");
+		builtin_error("invalid Broker data conversion for value argument", v);
 		return bro_broker::query_result();
 		}
 
@@ -282,15 +276,15 @@ function Broker::__get_index_from_value%(h: opaque of Broker::Store,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
-		return val_mgr->False();
+		builtin_error("invalid Broker store handle", h);
+		return bro_broker::query_result();
 		}
 
 	auto key = bro_broker::val_to_data(k);
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return bro_broker::query_result();
 		}
 
@@ -298,10 +292,10 @@ function Broker::__get_index_from_value%(h: opaque of Broker::Store,
 
 	if ( ! index )
 		{
-		builtin_error("invalid Broker data conversion for index argument");
+		builtin_error("invalid Broker data conversion for index argument", i);
 		return bro_broker::query_result();
 		}
-	
+
 	auto trigger = frame->GetTrigger();
 
 	if ( ! trigger )
@@ -336,8 +330,8 @@ function Broker::__keys%(h: opaque of Broker::Store%): Broker::QueryResult
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
-		return val_mgr->False();
+		builtin_error("invalid Broker store handle", h);
+		return bro_broker::query_result();
 		}
 
 	auto trigger = frame->GetTrigger();
@@ -374,7 +368,7 @@ function Broker::__put%(h: opaque of Broker::Store,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -383,13 +377,13 @@ function Broker::__put%(h: opaque of Broker::Store,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! val )
 		{
-		builtin_error("invalid Broker data conversion for value argument");
+		builtin_error("invalid Broker data conversion for value argument", v);
 		return val_mgr->False();
 		}
 
@@ -403,7 +397,7 @@ function Broker::__erase%(h: opaque of Broker::Store, k: any%): bool
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -411,7 +405,7 @@ function Broker::__erase%(h: opaque of Broker::Store, k: any%): bool
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
@@ -426,7 +420,7 @@ function Broker::__increment%(h: opaque of Broker::Store, k: any, a: any,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -435,13 +429,13 @@ function Broker::__increment%(h: opaque of Broker::Store, k: any, a: any,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! amount )
 		{
-		builtin_error("invalid Broker data conversion for amount argument");
+		builtin_error("invalid Broker data conversion for amount argument", a);
 		return val_mgr->False();
 		}
 
@@ -457,7 +451,7 @@ function Broker::__decrement%(h: opaque of Broker::Store, k: any, a: any,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -466,13 +460,13 @@ function Broker::__decrement%(h: opaque of Broker::Store, k: any, a: any,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! amount )
 		{
-		builtin_error("invalid Broker data conversion for amount argument");
+		builtin_error("invalid Broker data conversion for amount argument", a);
 		return val_mgr->False();
 		}
 
@@ -487,7 +481,7 @@ function Broker::__append%(h: opaque of Broker::Store, k: any, s: any,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -496,13 +490,13 @@ function Broker::__append%(h: opaque of Broker::Store, k: any, s: any,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! str )
 		{
-		builtin_error("invalid Broker data conversion for str argument");
+		builtin_error("invalid Broker data conversion for str argument", s);
 		return val_mgr->False();
 		}
 
@@ -517,7 +511,7 @@ function Broker::__insert_into_set%(h: opaque of Broker::Store, k: any, i: any,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -526,13 +520,13 @@ function Broker::__insert_into_set%(h: opaque of Broker::Store, k: any, i: any,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! idx )
 		{
-		builtin_error("invalid Broker data conversion for index argument");
+		builtin_error("invalid Broker data conversion for index argument", i);
 		return val_mgr->False();
 		}
 
@@ -548,7 +542,7 @@ function Broker::__insert_into_table%(h: opaque of Broker::Store, k: any,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -558,19 +552,19 @@ function Broker::__insert_into_table%(h: opaque of Broker::Store, k: any,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! idx )
 		{
-		builtin_error("invalid Broker data conversion for index argument");
+		builtin_error("invalid Broker data conversion for index argument", i);
 		return val_mgr->False();
 		}
 
 	if ( ! val )
 		{
-		builtin_error("invalid Broker data conversion for value argument");
+		builtin_error("invalid Broker data conversion for value argument", v);
 		return val_mgr->False();
 		}
 
@@ -586,7 +580,7 @@ function Broker::__remove_from%(h: opaque of Broker::Store, k: any, i: any,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -595,13 +589,13 @@ function Broker::__remove_from%(h: opaque of Broker::Store, k: any, i: any,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! idx )
 		{
-		builtin_error("invalid Broker data conversion for index argument");
+		builtin_error("invalid Broker data conversion for index argument", i);
 		return val_mgr->False();
 		}
 
@@ -617,7 +611,7 @@ function Broker::__push%(h: opaque of Broker::Store, k: any, v: any,
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -626,13 +620,13 @@ function Broker::__push%(h: opaque of Broker::Store, k: any, v: any,
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
 	if ( ! val )
 		{
-		builtin_error("invalid Broker data conversion for value argument");
+		builtin_error("invalid Broker data conversion for value argument", v);
 		return val_mgr->False();
 		}
 
@@ -646,7 +640,7 @@ function Broker::__pop%(h: opaque of Broker::Store, k: any, e: interval%): bool
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
@@ -654,7 +648,7 @@ function Broker::__pop%(h: opaque of Broker::Store, k: any, e: interval%): bool
 
 	if ( ! key )
 		{
-		builtin_error("invalid Broker data conversion for key argument");
+		builtin_error("invalid Broker data conversion for key argument", k);
 		return val_mgr->False();
 		}
 
@@ -668,11 +662,10 @@ function Broker::__clear%(h: opaque of Broker::Store%): bool
 
 	if ( ! handle )
 		{
-		builtin_error("invalid Broker store handle");
+		builtin_error("invalid Broker store handle", h);
 		return val_mgr->False();
 		}
 
-
 	handle->store.clear();
 	return val_mgr->True();
 	%}
diff --git a/testing/btest/Baseline/broker.store.invalid-handle/out b/testing/btest/Baseline/broker.store.invalid-handle/out
index 469583e41c..01aac30529 100644
--- a/testing/btest/Baseline/broker.store.invalid-handle/out
+++ b/testing/btest/Baseline/broker.store.invalid-handle/out
@@ -1,2 +1,3 @@
-error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/broker.store.invalid-handle/invalid-handle.zeek, line 6: invalid Broker store handle (Broker::keys(a))
-keys, F
+expression error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/broker.store.invalid-handle/invalid-handle.zeek, line 18: invalid Broker store handle (0), during call: (Broker::is_closed(a))
+error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/broker.store.invalid-handle/invalid-handle.zeek, line 6: invalid Broker store handle (Broker::keys(a) and 0)
+keys, [status=Broker::FAILURE, result=[data=<uninitialized>]]
diff --git a/testing/btest/broker/store/invalid-handle.zeek b/testing/btest/broker/store/invalid-handle.zeek
index c7e30addbe..c97669af60 100644
--- a/testing/btest/broker/store/invalid-handle.zeek
+++ b/testing/btest/broker/store/invalid-handle.zeek
@@ -13,8 +13,21 @@ function print_keys(a: any)
 		}
 	}
 
+function checkit(a: any)
+	{
+	if ( Broker::is_closed(a) )
+		print "this shouldn't get printed";
+	else
+		print "this shouldn't get printed either";
+	}
+
 global a: int = 0;
 
+event zeek_init() &priority=10
+	{
+	checkit(a);
+	}
+
 event zeek_init()
 	{
 	print_keys(a);