When overriding rules, ensure that the sources match

In places where a second rule definition might replace, append to, or replace items from a base rule, ensure that the source of the second rule definiton matches the first. This already existed for defines, but for other changes. There was a bug where a second definition might exist for a different source, but the additional rule was used anyway. This now returns the same error for these other changes e.g. "Rule has been re-defined..." as define. Signed-off-by: Mark Stemm <[email protected]>
falcosecurity · Oct 24, 2024 · 4a73ef8 · 4a73ef8
1 parent a44b311
commit 4a73ef8
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 6 deletions.
diff --git a/userspace/engine/rule_loader_collector.cpp b/userspace/engine/rule_loader_collector.cpp
@@ -182,10 +182,8 @@ void rule_loader::collector::append(configuration& cfg, macro_info& info) {
 }
 
 void rule_loader::collector::define(configuration& cfg, rule_info& info) {
-	const auto* prev = m_rule_infos.at(info.name);
-	THROW(prev && prev->source != info.source,
-	      "Rule has been re-defined with a different source",
-	      info.ctx);
+	auto prev = find_prev_rule(info);
+	(void)prev;
 
 	const auto* source = cfg.sources.at(info.source);
 	if(!source) {
@@ -205,7 +203,7 @@ void rule_loader::collector::define(configuration& cfg, rule_info& info) {
 }
 
 void rule_loader::collector::append(configuration& cfg, rule_update_info& info) {
-	auto prev = m_rule_infos.at(info.name);
+	auto prev = find_prev_rule(info);
 
 	THROW(!prev, ERROR_NO_PREVIOUS_RULE_APPEND, info.ctx);
 	THROW(!info.has_any_value(),
@@ -275,7 +273,7 @@ void rule_loader::collector::append(configuration& cfg, rule_update_info& info)
 }
 
 void rule_loader::collector::selective_replace(configuration& cfg, rule_update_info& info) {
-	auto prev = m_rule_infos.at(info.name);
+	auto prev = find_prev_rule(info);
 
 	THROW(!prev, ERROR_NO_PREVIOUS_RULE_REPLACE, info.ctx);
 	THROW(!info.has_any_value(),
@@ -330,6 +328,19 @@ void rule_loader::collector::selective_replace(configuration& cfg, rule_update_i
 	replace_info(prev, info, m_cur_index++);
 }
 
+template<typename ruleInfo>
+rule_loader::rule_info* rule_loader::collector::find_prev_rule(ruleInfo& info) {
+	auto ret = m_rule_infos.at(info.name);
+
+	// Throw an error if both the original rule and current rule
+	// have the same name and explicitly have different sources.
+	THROW(ret && (ret->source != "" && info.source != "" && ret->source != info.source),
+	      "Rule has been re-defined with a different source",
+	      info.ctx);
+
+	return ret;
+}
+
 void rule_loader::collector::enable(configuration& cfg, rule_info& info) {
 	auto prev = m_rule_infos.at(info.name);
 	THROW(!prev, "Rule has 'enabled' key but no rule by that name already exists", info.ctx);

diff --git a/userspace/engine/rule_loader_collector.h b/userspace/engine/rule_loader_collector.h
@@ -97,6 +97,9 @@ class collector {
 	virtual void selective_replace(configuration& cfg, rule_update_info& info);
 
 private:
+	template<typename ruleInfo>
+	rule_info* find_prev_rule(ruleInfo& info);
+
 	uint32_t m_cur_index;
 	indexed_vector<rule_info> m_rule_infos;
 	indexed_vector<macro_info> m_macro_infos;