valitydev/osquery-1
14
0
Fork 0
mirror of https://github.com/valitydev/osquery-1.git synced 2024-11-06 17:45:22 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix parsing an invalid decorators config (#6317)

Browse Source 
The "decorators" configuration value must be a JSON object,
otherwise we try to search through its inexistent members
and dereference a null pointer.

Added also a regression test.

Addresses https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19274
This commit is contained in:
Stefano Bonicatti 2020-03-20 13:39:08 +01:00 committed by GitHub
parent 0409360ace
commit 2e84e8cdf1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 42 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
plugins/config/parsers/decorators.cpp
View File

@ -6,13 +6,13 @@
* the LICENSE file found in the root directory of this source tree.
*/
#include <plugins/config/parsers/decorators.h>
#include <osquery/config/config.h>
#include <osquery/flags.h>
#include <osquery/logger.h>
#include <osquery/registry_factory.h>
#include <osquery/sql.h>
#include <osquery/utils/json/json.h>
#include <plugins/config/parsers/decorators.h>
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<std::string, std::string>& results) {
REGISTER_INTERNAL(DecoratorsConfigParserPlugin,
"config_parser",
kDecorationsName.c_str());
}
} // namespace osquery

34
plugins/config/parsers/tests/decorators_tests.cpp
View File

@ -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<std::string, std::string> decorators_as_array = {
{"decorators", "[1,2,3]"}};
auto status = Config::get().update(decorators_as_array);
ASSERT_FALSE(status.ok());
std::map<std::string, std::string> decorators_as_string = {
{"decorators", "abc"}};
status = Config::get().update(decorators_as_string);
ASSERT_FALSE(status.ok());
std::map<std::string, std::string> decorators_as_number = {
{"decorators", "1"}};
status = Config::get().update(decorators_as_number);
ASSERT_FALSE(status.ok());
}
} // namespace osquery