From 421639e7a7f9736155e1e06c22d8b69a15ef75cf Mon Sep 17 00:00:00 2001 From: Christian Kreibich Date: Fri, 8 Jan 2021 19:13:06 -0800 Subject: [PATCH 1/3] Explicitly don't support sets with multiple index types in input/config frameworks The input framework's Manager::IsCompatibleType() already rejected sets with multiple index types that aren't all the same (i.e. that are not pure). Pure ones (e.g. "set[addr,addr]") slipped through and could cause Zeek to segfault elsewhere in the config framework due to type comparison subtleties. Note that the ASCII reader can't read such sets anyway, so this method now rejects sets with any kind of index-type tuple. In the config framework, the script-level change handler has a risky conversion from any to set[bool], which can trigger segfaults when the underlying set's index is a type tuple. We now prevent this code path by ensuring it only applies to sets with a single index type. --- scripts/base/frameworks/config/main.zeek | 7 ++++++- src/input/Manager.cc | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/base/frameworks/config/main.zeek b/scripts/base/frameworks/config/main.zeek index e2cfb11c8a..5fe7f6eaaf 100644 --- a/scripts/base/frameworks/config/main.zeek +++ b/scripts/base/frameworks/config/main.zeek @@ -124,8 +124,13 @@ function format_value(value: any) : string { local tn = type_name(value); local part: string_vec = vector(); - if ( /^set/ in tn ) + + if ( /^set/ in tn && strstr(tn, ",") == 0 ) { + # The conversion to set here is tricky and assumes + # that the set isn't indexed via a tuple of types. + # The above check for commas in the type name + # ensures this. local it: set[bool] = value; for ( sv in it ) part += cat(sv); diff --git a/src/input/Manager.cc b/src/input/Manager.cc index 27b47491eb..e065b87566 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -833,7 +833,12 @@ bool Manager::IsCompatibleType(Type* t, bool atomic_only) if ( ! t->IsSet() ) return false; - return IsCompatibleType(t->AsSetType()->GetIndices()->GetPureType().get(), true); + const auto& indices = t->AsSetType()->GetIndices(); + + if ( indices->GetTypes().size() != 1 ) + return false; + + return IsCompatibleType(indices->GetPureType().get(), true); } case TYPE_VECTOR: From aa9242913fbe26a7c29218921ce99f06a6b7499c Mon Sep 17 00:00:00 2001 From: Christian Kreibich Date: Fri, 8 Jan 2021 19:23:37 -0800 Subject: [PATCH 2/3] More precise type information in a config framework error message When an option's value is a reader-incompatible table or set, Zeek now renders the type as expressed in the script layer (e.g. "set[addr,addr]") as opposed to the internal type tag (which'd here be "table", including for sets). --- src/input/readers/config/Config.cc | 9 ++++++--- src/input/readers/config/Config.h | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/input/readers/config/Config.cc b/src/input/readers/config/Config.cc index 0a3d33811c..ec6226de99 100644 --- a/src/input/readers/config/Config.cc +++ b/src/input/readers/config/Config.cc @@ -10,6 +10,7 @@ #include #include +#include "zeek/Desc.h" #include "zeek/input/Manager.h" #include "zeek/threading/SerialTypes.h" @@ -38,7 +39,7 @@ Config::Config(ReaderFrontend *frontend) : ReaderBackend(frontend) if ( id->GetType()->Tag() == TYPE_RECORD || ! Manager::IsCompatibleType(id->GetType().get()) ) { - option_types[id->Name()] = std::make_tuple(TYPE_ERROR, id->GetType()->Tag()); + option_types[id->Name()] = std::make_tuple(TYPE_ERROR, id->GetType()->Tag(), id); continue; } @@ -49,7 +50,7 @@ Config::Config(ReaderFrontend *frontend) : ReaderBackend(frontend) else if ( primary == TYPE_VECTOR ) secondary = id->GetType()->AsVectorType()->Yield()->Tag(); - option_types[id->Name()] = std::make_tuple(primary, secondary); + option_types[id->Name()] = std::make_tuple(primary, secondary, id); } } @@ -212,8 +213,10 @@ bool Config::DoUpdate() if ( std::get<0>((*typeit).second) == TYPE_ERROR ) { + ODesc d; + std::get<2>((*typeit).second)->GetType()->Describe(&d); Warning(Fmt("Option '%s' has type '%s', which is not supported for file input. Ignoring line.", - key.c_str(), type_name(std::get<1>((*typeit).second)))); + key.c_str(), d.Description())); continue; } diff --git a/src/input/readers/config/Config.h b/src/input/readers/config/Config.h index c8e8360ccd..fe525c66ad 100644 --- a/src/input/readers/config/Config.h +++ b/src/input/readers/config/Config.h @@ -9,6 +9,7 @@ #include #include +#include "zeek/ID.h" #include "zeek/input/ReaderBackend.h" #include "zeek/threading/formatters/Ascii.h" @@ -50,7 +51,7 @@ private: std::string empty_field; std::unique_ptr formatter; - std::unordered_map> option_types; + std::unordered_map> option_types; std::unordered_map option_values; }; From fcab8df8fb1de94484e552cd8bc83dd9a52e66c7 Mon Sep 17 00:00:00 2001 From: Christian Kreibich Date: Fri, 8 Jan 2021 19:25:41 -0800 Subject: [PATCH 3/3] Btest tweak for improved type rendering in config framework errors and set types --- .../Baseline/scripts.base.frameworks.input.config.errors/errout | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.config.errors/errout b/testing/btest/Baseline/scripts.base.frameworks.input.config.errors/errout index 8f72c4b0c2..386257a5df 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.config.errors/errout +++ b/testing/btest/Baseline/scripts.base.frameworks.input.config.errors/errout @@ -10,6 +10,6 @@ warning: Value 'unknown' for source 'thread ../configfile/Input::READER_CONFIG' error: SendEvent for event InputConfig::new_value failed warning: ../configfile/Input::READER_CONFIG: Option 'testbooool' does not exist. Ignoring line. warning: ../configfile/Input::READER_CONFIG: Option 'test_any' has type 'any', which is not supported for file input. Ignoring line. -warning: ../configfile/Input::READER_CONFIG: Option 'test_table' has type 'table', which is not supported for file input. Ignoring line. +warning: ../configfile/Input::READER_CONFIG: Option 'test_table' has type 'table[string] of string', which is not supported for file input. Ignoring line. received termination signal >>>