diff --git a/CHANGES b/CHANGES index 47137cfd53..421a81baf8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,21 @@ +6.0.0-dev.391 | 2023-04-19 19:48:50 +0200 + + * zeek.bif: Remove cat_sep() fully var_arg changes (Arne Welzel, Corelight) + + These were introduced to better catch type violations at runtime. With + bifcl doing these checks, revert to a better documented version. + + * GH-2935: broker/messaging: Runtime type checks for pool (Arne Welzel, Corelight) + + publish_hrw() and publish_rr() are excluded from type checking due to their + variadic nature. Passing a wrong type for the pool argument previously triggered + an abort, now the result is runtime errors. This isn't great, but it's + better than crashing Zeek. + + Closes #2935 + + * bifcl: Bump for runtime-type checks in var_arg bifs (Arne Welzel, Corelight) + 6.0.0-dev.387 | 2023-04-19 09:01:32 -0700 * Add call stacks to script profiler output (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index ff2aa57cb9..a6acaf1fb8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.0.0-dev.387 +6.0.0-dev.391 diff --git a/auxil/bifcl b/auxil/bifcl index 14f393117f..b6f138be79 160000 --- a/auxil/bifcl +++ b/auxil/bifcl @@ -1 +1 @@ -Subproject commit 14f393117f63ef7327a40ade435f9021787b4f45 +Subproject commit b6f138be79f7d4408302b1297b0c63092b019773 diff --git a/src/broker/messaging.bif b/src/broker/messaging.bif index 4997177bc7..e647928190 100644 --- a/src/broker/messaging.bif +++ b/src/broker/messaging.bif @@ -65,6 +65,16 @@ static bool publish_event_args(zeek::ValPList& args, const zeek::String* topic, return rval; } +static bool is_cluster_pool(zeek::Val* pool) + { + static zeek::RecordTypePtr pool_type = nullptr; + + if ( ! pool_type ) + pool_type = zeek::id::find_type("Cluster::Pool"); + + return pool->GetType() == pool_type; + } + %%} module Broker; @@ -186,6 +196,12 @@ function Cluster::publish_rr%(pool: Pool, key: string, ...%): bool if ( ! topic_func ) topic_func = zeek::detail::global_scope()->Find("Cluster::rr_topic")->GetVal()->AsFunc(); + if ( ! is_cluster_pool(pool) ) + { + zeek::emit_builtin_error("expected type Cluster::Pool for pool"); + return zeek::val_mgr->False(); + } + zeek::Args vl{{zeek::NewRef{}, pool}, {zeek::NewRef{}, key}}; auto topic = topic_func->Invoke(&vl); @@ -223,6 +239,12 @@ function Cluster::publish_hrw%(pool: Pool, key: any, ...%): bool if ( ! topic_func ) topic_func = zeek::detail::global_scope()->Find("Cluster::hrw_topic")->GetVal()->AsFunc(); + if ( ! is_cluster_pool(pool) ) + { + zeek::emit_builtin_error("expected type Cluster::Pool for pool"); + return zeek::val_mgr->False(); + } + zeek::Args vl{{zeek::NewRef{}, pool}, {zeek::NewRef{}, key}}; auto topic = topic_func->Invoke(&vl); diff --git a/src/zeek.bif b/src/zeek.bif index 5684d7fac5..924cb720e1 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -1541,44 +1541,18 @@ function cat%(...%): string ## Concatenates all arguments, with a separator placed between each one. This ## function is similar to :zeek:id:`cat`, but places a separator between each ## given argument. If any of the variable arguments is an empty string it is -## replaced by the given default string instead. This function takes at least -## two arguments. The first argument is the separator *sep* placed between -## each argument, the second argument is the default string *def* used when -## an argument is the empty string. +## replaced by the given default string instead. +## +## sep: The separator to place between each argument. +## +## def: The default string to use when an argument is the empty string. ## ## Returns: A concatenation of all arguments with *sep* between each one and ## empty strings replaced with *def*. ## ## .. zeek:see:: cat string_cat -function cat_sep%(...%): string +function cat_sep%(sep: string, def: string, ...%): string %{ - if ( @ARGC@ < 2 ) - { - zeek::emit_builtin_error(zeek::util::fmt("cat_sep() takes at least 2 arguments, got %ld", - @ARGC@)); - return nullptr; - } - - zeek::ValPtr sep_arg = @ARG@[0]; - zeek::ValPtr def_arg = @ARG@[1]; - - if ( sep_arg->GetType()->Tag() != zeek::TYPE_STRING ) - { - zeek::emit_builtin_error(zeek::util::fmt("expected type string for separator, got %s", - zeek::type_name(sep_arg->GetType()->Tag()))); - return nullptr; - } - - if ( def_arg->GetType()->Tag() != zeek::TYPE_STRING ) - { - zeek::emit_builtin_error(zeek::util::fmt("expected type string for default, got %s", - zeek::type_name(def_arg->GetType()->Tag()))); - return nullptr; - } - - zeek::StringVal* sep = sep_arg->AsStringVal(); - zeek::StringVal* def = def_arg->AsStringVal(); - zeek::ODesc d; d.SetStyle(RAW_STYLE); int pre_size = 0; diff --git a/testing/btest/Baseline/bifs.cat_sep_errors-2/.stderr b/testing/btest/Baseline/bifs.cat_sep_errors-2/.stderr index 13a40bc019..f3eeb2d94b 100644 --- a/testing/btest/Baseline/bifs.cat_sep_errors-2/.stderr +++ b/testing/btest/Baseline/bifs.cat_sep_errors-2/.stderr @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/cat_sep_errors.zeek, line 3: cat_sep() takes at least 2 arguments, got 1 (cat_sep(sep)) +error in <...>/cat_sep_errors.zeek, line 3: cat_sep() takes at least 2 argument(s), got 1 (cat_sep(sep)) diff --git a/testing/btest/Baseline/bifs.cat_sep_errors-3/.stderr b/testing/btest/Baseline/bifs.cat_sep_errors-3/.stderr index c19b73c3da..3aec3d0c30 100644 --- a/testing/btest/Baseline/bifs.cat_sep_errors-3/.stderr +++ b/testing/btest/Baseline/bifs.cat_sep_errors-3/.stderr @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/cat_sep_errors.zeek, line 4: expected type string for default, got count (cat_sep(sep, 1)) +error in <...>/cat_sep_errors.zeek, line 4: expected type string for def, got count (cat_sep(sep, 1)) diff --git a/testing/btest/Baseline/bifs.cat_sep_errors-4/.stderr b/testing/btest/Baseline/bifs.cat_sep_errors-4/.stderr index 6416ec7c35..c414809740 100644 --- a/testing/btest/Baseline/bifs.cat_sep_errors-4/.stderr +++ b/testing/btest/Baseline/bifs.cat_sep_errors-4/.stderr @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/cat_sep_errors.zeek, line 4: expected type string for separator, got count (cat_sep(1, default)) +error in <...>/cat_sep_errors.zeek, line 4: expected type string for sep, got count (cat_sep(1, default)) diff --git a/testing/btest/Baseline/bifs.cat_sep_errors-5/.stderr b/testing/btest/Baseline/bifs.cat_sep_errors-5/.stderr index 53fbad9616..fd3e37d3a7 100644 --- a/testing/btest/Baseline/bifs.cat_sep_errors-5/.stderr +++ b/testing/btest/Baseline/bifs.cat_sep_errors-5/.stderr @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/cat_sep_errors.zeek, line 3: expected type string for separator, got record (cat_sep([$a=1], default)) +error in <...>/cat_sep_errors.zeek, line 3: expected type string for sep, got record (cat_sep([$a=1], default)) diff --git a/testing/btest/Baseline/bifs.cat_sep_errors/.stderr b/testing/btest/Baseline/bifs.cat_sep_errors/.stderr index f3813297fe..c0a49beadf 100644 --- a/testing/btest/Baseline/bifs.cat_sep_errors/.stderr +++ b/testing/btest/Baseline/bifs.cat_sep_errors/.stderr @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/cat_sep_errors.zeek, line 6: cat_sep() takes at least 2 arguments, got 0 (cat_sep()) +error in <...>/cat_sep_errors.zeek, line 6: cat_sep() takes at least 2 argument(s), got 0 (cat_sep()) diff --git a/testing/btest/Baseline/scripts.base.frameworks.cluster.publish-hrw-type-check/.stderr b/testing/btest/Baseline/scripts.base.frameworks.cluster.publish-hrw-type-check/.stderr new file mode 100644 index 0000000000..436f13c613 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.cluster.publish-hrw-type-check/.stderr @@ -0,0 +1,5 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in <...>/publish-hrw-type-check.zeek, line 15: expected type Cluster::Pool for pool (Cluster::publish_hrw(/topic, 1234/tcp, test_send, [$a=a])) +error in <...>/publish-hrw-type-check.zeek, line 17: expected type Cluster::Pool for pool (Cluster::publish_hrw(0, 1234/tcp, test_send, [$a=a])) +error in <...>/publish-hrw-type-check.zeek, line 19: expected type Cluster::Pool for pool (Cluster::publish_rr(/topic, val, test_send, [$a=b])) +error in <...>/publish-hrw-type-check.zeek, line 21: expected type string for key, got port (Cluster::publish_rr(Cluster::Pool(), 1234/tcp, test_send, [$a=c])) diff --git a/testing/btest/scripts/base/frameworks/cluster/publish-hrw-type-check.zeek b/testing/btest/scripts/base/frameworks/cluster/publish-hrw-type-check.zeek new file mode 100644 index 0000000000..7c9dabde16 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/cluster/publish-hrw-type-check.zeek @@ -0,0 +1,22 @@ +# @TEST-DOC: Check that Cluster::publish_hrw() and Cluster::publish_rr() do not cause an abort when provided with wrongly typed arguments. +# @TEST-EXEC: zeek -b %INPUT +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr + +@load base/frameworks/cluster + +type R: record { + a: string; +}; + +event test_send(r: R) { } + +event zeek_init() + { + Cluster::publish_hrw("/topic", 1234/tcp, test_send, [$a="a"]); + + Cluster::publish_hrw(0, 1234/tcp, test_send, [$a="a"]); + + Cluster::publish_rr("/topic", "val", test_send, [$a="b"]); + + Cluster::publish_rr(Cluster::Pool(), 1234/tcp, test_send, [$a="c"]); + }