rules: don't load rules that fail to compile

Be sure that we don't load invalid regexp rules. related: #536
evilsocket · Nov 22, 2021 · 9821800 · 9821800
1 parent 8037c38
commit 9821800
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 0 deletions.
diff --git a/daemon/rule/loader.go b/daemon/rule/loader.go
@@ -116,11 +116,13 @@ func (l *Loader) loadRule(fileName string) error {
 	if r.Enabled {
 		if err := r.Operator.Compile(); err != nil {
 			log.Warning("Operator.Compile() error: %s: %s", err, r.Operator.Data)
+			return fmt.Errorf("(1) Error compiling rule: %s", err)
 		}
 		if r.Operator.Type == List {
 			for i := 0; i < len(r.Operator.List); i++ {
 				if err := r.Operator.List[i].Compile(); err != nil {
 					log.Warning("Operator.Compile() error: %s: ", err)
+					return fmt.Errorf("(1) Error compiling list rule: %s", err)
 				}
 			}
 		}
@@ -275,6 +277,7 @@ func (l *Loader) replaceUserRule(rule *Rule) (err error) {
 	if rule.Enabled {
 		if err := rule.Operator.Compile(); err != nil {
 			log.Warning("Operator.Compile() error: %s: %s", err, rule.Operator.Data)
+			return fmt.Errorf("(2) Error compiling rule: %s", err)
 		}
 
 		if rule.Operator.Type == List {
@@ -286,6 +289,7 @@ func (l *Loader) replaceUserRule(rule *Rule) (err error) {
 			for i := 0; i < len(rule.Operator.List); i++ {
 				if err := rule.Operator.List[i].Compile(); err != nil {
 					log.Warning("Operator.Compile() error: %s: ", err)
+					return fmt.Errorf("(2) Error compiling list rule: %s", err)
 				}
 			}
 		}

diff --git a/daemon/rule/loader_test.go b/daemon/rule/loader_test.go
@@ -16,6 +16,7 @@ func TestMain(m *testing.M) {
 	defer os.RemoveAll(tmpDir)
 	os.Exit(m.Run())
 }
+
 func TestRuleLoader(t *testing.T) {
 	t.Parallel()
 	t.Log("Test rules loader")
@@ -61,6 +62,39 @@ func TestRuleLoader(t *testing.T) {
 	testDurationChange(t, l)
 }
 
+func TestRuleLoaderInvalidRegexp(t *testing.T) {
+	t.Parallel()
+	t.Log("Test rules loader: invalid regexp")
+
+	l, err := NewLoader(true)
+	if err != nil {
+		t.Fail()
+	}
+	t.Run("loadRule() from disk test (simple)", func(t *testing.T) {
+		if err := l.loadRule("testdata/invalid-regexp.json"); err == nil {
+			t.Error("invalid regexp rule loaded: loadRule()")
+		}
+	})
+
+	t.Run("loadRule() from disk test (list)", func(t *testing.T) {
+		if err := l.loadRule("testdata/invalid-regexp-list.json"); err == nil {
+			t.Error("invalid regexp rule loaded: loadRule()")
+		}
+	})
+
+	var list []Operator
+	dur30m := Duration("30m")
+	opListData := `[{"type": "regexp", "operand": "process.path", "sensitive": false, "data": "^(/di(rmngr)$"}, {"type": "simple", "operand": "dest.port", "data": "53", "sensitive": false}]`
+	invalidRegexpOp, _ := NewOperator(List, false, OpList, opListData, list)
+	invalidRegexpRule := Create("invalid-regexp", true, false, Allow, dur30m, invalidRegexpOp)
+
+	t.Run("replaceUserRule() test list", func(t *testing.T) {
+		if err := l.replaceUserRule(invalidRegexpRule); err == nil {
+			t.Error("invalid regexp rule loaded: replaceUserRule()")
+		}
+	})
+}
+
 func TestLiveReload(t *testing.T) {
 	t.Parallel()
 	t.Log("Test rules loader with live reload")

diff --git a/daemon/rule/operator_test.go b/daemon/rule/operator_test.go
@@ -276,6 +276,23 @@ func TestNewOperatorRegexp(t *testing.T) {
 	restoreConnection()
 }
 
+func TestNewOperatorInvalidRegexp(t *testing.T) {
+	t.Log("Test NewOperator() invalid regexp")
+	var dummyList []Operator
+
+	opRE, err := NewOperator(Regexp, false, OpProto, "^TC(P$", dummyList)
+	if err != nil {
+		t.Error("NewOperator regexp.err should be nil: ", err)
+		t.Fail()
+	}
+	if err = opRE.Compile(); err == nil {
+		t.Error("NewOperator() invalid regexp. It should fail: ", err)
+		t.Fail()
+	}
+
+	restoreConnection()
+}
+
 func TestNewOperatorRegexpSensitive(t *testing.T) {
 	t.Log("Test NewOperator() regexp sensitive")
 	var dummyList []Operator

diff --git a/daemon/rule/testdata/invalid-regexp-list.json b/daemon/rule/testdata/invalid-regexp-list.json
@@ -0,0 +1,31 @@
+{
+  "created": "2020-12-13T18:06:52.209804547+01:00",
+  "updated": "2020-12-13T18:06:52.209857713+01:00",
+  "name": "invalid-regexp-list",
+  "enabled": true,
+  "precedence": true,
+  "action": "allow",
+  "duration": "always",
+  "operator": {
+    "type": "list",
+    "operand": "list",
+    "sensitive": false,
+    "data": "[{\"type\": \"regexp\", \"operand\": \"process.path\", \"sensitive\": false, \"data\": \"^(/di(rmngr$\"}, {\"type\": \"simple\", \"operand\": \"dest.port\", \"data\": \"53\", \"sensitive\": false}]",
+    "list": [
+        {
+        "type": "regexp",
+        "operand": "process.path",
+        "sensitive": false,
+        "data": "^(/di(rmngr)$",
+        "list": null
+      },
+      {
+        "type": "simple",
+        "operand": "dest.port",
+        "sensitive": false,
+        "data": "53",
+        "list": null
+      }
+    ]
+  }
+}
diff --git a/daemon/rule/testdata/invalid-regexp.json b/daemon/rule/testdata/invalid-regexp.json
@@ -0,0 +1,16 @@
+{
+  "created": "2020-12-13T18:06:52.209804547+01:00",
+  "updated": "2020-12-13T18:06:52.209857713+01:00",
+  "name": "invalid-regexp",
+  "enabled": true,
+  "precedence": true,
+  "action": "allow",
+  "duration": "always",
+  "operator": {
+    "type": "regexp",
+    "operand": "process.path",
+    "sensitive": false,
+    "data": "/opt/((.*)google/chrome/chrome",
+    "list": []
+  }
+}