diff --git a/plugins/config/parsers/decorators.cpp b/plugins/config/parsers/decorators.cpp index 6d36e349..2b40a67a 100644 --- a/plugins/config/parsers/decorators.cpp +++ b/plugins/config/parsers/decorators.cpp @@ -6,13 +6,13 @@ * the LICENSE file found in the root directory of this source tree. */ -#include #include #include #include #include #include #include +#include namespace osquery { @@ -99,7 +99,7 @@ class DecoratorsConfigParserPlugin : public ConfigParserPlugin { /// Protect the configuration controlled content. static Mutex kDecorationsConfigMutex; }; -} +} // namespace DecorationStore DecoratorsConfigParserPlugin::kDecorations; Mutex DecoratorsConfigParserPlugin::kDecorationsMutex; @@ -117,6 +117,16 @@ Status DecoratorsConfigParserPlugin::update(const std::string& source, clearDecorations(source); auto decorations = config.find(kDecorationsName); if (decorations != config.end()) { + if (!decorations->second.doc().IsObject()) { + const auto error_message = + "Invalid format for decorators configuration, decorators value " + "must be a JSON " + "object"; + LOG(WARNING) << error_message; + + return Status::failure(error_message); + } + // Each of these methods acquires the decorator lock separately. // The run decorators method is designed to have call sites throughout // the code base. @@ -310,4 +320,4 @@ void getDecorations(std::map& results) { REGISTER_INTERNAL(DecoratorsConfigParserPlugin, "config_parser", kDecorationsName.c_str()); -} +} // namespace osquery diff --git a/plugins/config/parsers/tests/decorators_tests.cpp b/plugins/config/parsers/tests/decorators_tests.cpp index e45e0ced..18c41e1c 100644 --- a/plugins/config/parsers/tests/decorators_tests.cpp +++ b/plugins/config/parsers/tests/decorators_tests.cpp @@ -60,7 +60,9 @@ class DecoratorsConfigParserPluginTests : public testing::Test { TEST_F(DecoratorsConfigParserPluginTests, test_decorators_list) { // Assume the decorators are disabled. - Config::get().update(config_data_); + auto status = Config::get().update(config_data_); + ASSERT_TRUE(status.ok()) << status.getMessage(); + auto parser = Config::getParser("decorators"); EXPECT_NE(parser, nullptr); @@ -74,7 +76,8 @@ TEST_F(DecoratorsConfigParserPluginTests, test_decorators_run_load) { // Re-enable the decorators, then update the config. // The 'load' decorator set should run every time the config is updated. FLAGS_disable_decorators = false; - Config::get().update(config_data_); + auto status = Config::get().update(config_data_); + ASSERT_TRUE(status.ok()) << status.getMessage(); QueryLogItem item; getDecorations(item.decorations); @@ -85,7 +88,8 @@ TEST_F(DecoratorsConfigParserPluginTests, test_decorators_run_load) { TEST_F(DecoratorsConfigParserPluginTests, test_decorators_run_interval) { // Prevent loads from executing. FLAGS_disable_decorators = true; - Config::get().update(config_data_); + auto status = Config::get().update(config_data_); + ASSERT_TRUE(status.ok()) << status.getMessage(); // Mimic the schedule's execution. FLAGS_disable_decorators = false; @@ -123,7 +127,8 @@ TEST_F(DecoratorsConfigParserPluginTests, test_decorators_run_load_top_level) { FLAGS_disable_decorators = false; // enable top level decorations for the test FLAGS_decorations_top_level = true; - Config::get().update(config_data_); + auto status = Config::get().update(config_data_); + ASSERT_TRUE(status.ok()) << status.getMessage(); // make sure decorations object still exists QueryLogItem item; @@ -133,7 +138,7 @@ TEST_F(DecoratorsConfigParserPluginTests, test_decorators_run_load_top_level) { // serialize the QueryLogItem and make sure decorations go top level auto doc = JSON::newObject(); - auto status = serializeQueryLogItem(item, doc); + status = serializeQueryLogItem(item, doc); std::string expected = "test"; std::string result = doc.doc()["load_test"].GetString(); EXPECT_EQ(result, expected); @@ -141,4 +146,23 @@ TEST_F(DecoratorsConfigParserPluginTests, test_decorators_run_load_top_level) { // disable top level decorations FLAGS_decorations_top_level = false; } +TEST_F(DecoratorsConfigParserPluginTests, test_invalid_decorators) { + // Prevent loads from executing. + FLAGS_disable_decorators = true; + + std::map decorators_as_array = { + {"decorators", "[1,2,3]"}}; + auto status = Config::get().update(decorators_as_array); + ASSERT_FALSE(status.ok()); + + std::map decorators_as_string = { + {"decorators", "abc"}}; + status = Config::get().update(decorators_as_string); + ASSERT_FALSE(status.ok()); + + std::map decorators_as_number = { + {"decorators", "1"}}; + status = Config::get().update(decorators_as_number); + ASSERT_FALSE(status.ok()); +} } // namespace osquery