Add throw for empty strings in rules with modifier contains, startwit…

…h, and endswith (opensearch-project#860) (opensearch-project#896) * add validation for empty strings with contains, startswith and endswith modifiers * throw exception if empty string with contains, startswith, or endswith * change var name * add modifiers to log --------- Signed-off-by: Joanne Wang <[email protected]>
goyamegh · Mar 8, 2024 · ad3f5a5 · ad3f5a5
1 parent 26c06e7
commit ad3f5a5
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 1 deletion.
diff --git a/src/main/java/org/opensearch/securityanalytics/rules/objects/SigmaDetectionItem.java b/src/main/java/org/opensearch/securityanalytics/rules/objects/SigmaDetectionItem.java
@@ -18,6 +18,7 @@
 import org.opensearch.securityanalytics.rules.modifiers.SigmaModifierFacade;
 import org.opensearch.securityanalytics.rules.modifiers.SigmaValueModifier;
 import org.opensearch.securityanalytics.rules.types.SigmaNull;
+import org.opensearch.securityanalytics.rules.types.SigmaString;
 import org.opensearch.securityanalytics.rules.types.SigmaType;
 import org.opensearch.securityanalytics.rules.types.SigmaTypeFacade;
 import org.opensearch.securityanalytics.rules.utils.AnyOneOf;
@@ -111,7 +112,14 @@ public static <T> SigmaDetectionItem fromMapping(String key, Either<T, List<T>>
 
         List<SigmaType> sigmaTypes = new ArrayList<>();
         for (T v: values) {
-            sigmaTypes.add(SigmaTypeFacade.sigmaType(v));
+            SigmaType sigmaType = SigmaTypeFacade.sigmaType(v);
+            // throws an error if sigmaType is an empty string and the modifier is "contains" or "startswith" or "endswith"
+            boolean invalidModifierWithEmptyString = modifierIds.contains("contains") || modifierIds.contains("startswith") || modifierIds.contains("endswith");
+            if (sigmaType.getClass().equals(SigmaString.class) && v.toString().isEmpty() && invalidModifierWithEmptyString) {
+                throw new SigmaValueError("Cannot create rule with empty string and given modifier(s): " + modifierIds);
+            } else {
+                sigmaTypes.add(sigmaType);
+            }
         }
 
         return new SigmaDetectionItem(field, modifiers, sigmaTypes, null, null, true);

diff --git a/src/test/java/org/opensearch/securityanalytics/rules/backend/QueryBackendTests.java b/src/test/java/org/opensearch/securityanalytics/rules/backend/QueryBackendTests.java
@@ -907,6 +907,78 @@ public void testConvertUnboundValuesAsWildcard() throws IOException, SigmaError
         Assert.assertEquals("((mappedA: \"value1\") OR (mappedA: \"value2\") OR (mappedA: \"value3\")) OR (test*)", queries.get(0).toString());
     }
 
+    public void testConvertSkipEmptyStringStartsWithModifier() throws IOException, SigmaError {
+        OSQueryBackend queryBackend = testBackend();
+        Assert.assertThrows(SigmaValueError.class, () -> {
+            queryBackend.convertRule(SigmaRule.fromYaml(
+                    "            title: Test\n" +
+                            "            id: 39f919f3-980b-4e6f-a975-8af7e507ef2b\n" +
+                            "            status: test\n" +
+                            "            level: critical\n" +
+                            "            description: Detects QuarksPwDump clearing access history in hive\n" +
+                            "            author: Florian Roth\n" +
+                            "            date: 2017/05/15\n" +
+                            "            logsource:\n" +
+                            "                category: test_category\n" +
+                            "                product: test_product\n" +
+                            "            detection:\n" +
+                            "                sel:\n" +
+                            "                    fieldA1|startswith: \n" +
+                            "                        - value1\n" +
+                            "                        - value2\n" +
+                            "                        - ''\n" +
+                            "                condition: sel", false));
+        });
+    }
+
+    public void testConvertSkipEmptyStringEndsWithModifier() throws IOException, SigmaError {
+        OSQueryBackend queryBackend = testBackend();
+        Assert.assertThrows(SigmaValueError.class, () -> {
+            queryBackend.convertRule(SigmaRule.fromYaml(
+                    "            title: Test\n" +
+                            "            id: 39f919f3-980b-4e6f-a975-8af7e507ef2b\n" +
+                            "            status: test\n" +
+                            "            level: critical\n" +
+                            "            description: Detects QuarksPwDump clearing access history in hive\n" +
+                            "            author: Florian Roth\n" +
+                            "            date: 2017/05/15\n" +
+                            "            logsource:\n" +
+                            "                category: test_category\n" +
+                            "                product: test_product\n" +
+                            "            detection:\n" +
+                            "                sel:\n" +
+                            "                    fieldA1|endswith: \n" +
+                            "                        - value1\n" +
+                            "                        - value2\n" +
+                            "                        - ''\n" +
+                            "                condition: sel", false));
+        });
+    }
+
+    public void testConvertSkipEmptyStringContainsModifier() throws IOException, SigmaError {
+        OSQueryBackend queryBackend = testBackend();
+        Assert.assertThrows(SigmaValueError.class, () -> {
+            queryBackend.convertRule(SigmaRule.fromYaml(
+                    "            title: Test\n" +
+                            "            id: 39f919f3-980b-4e6f-a975-8af7e507ef2b\n" +
+                            "            status: test\n" +
+                            "            level: critical\n" +
+                            "            description: Detects QuarksPwDump clearing access history in hive\n" +
+                            "            author: Florian Roth\n" +
+                            "            date: 2017/05/15\n" +
+                            "            logsource:\n" +
+                            "                category: test_category\n" +
+                            "                product: test_product\n" +
+                            "            detection:\n" +
+                            "                sel:\n" +
+                            "                    fieldA1|contains: \n" +
+                            "                        - value1\n" +
+                            "                        - value2\n" +
+                            "                        - ''\n" +
+                            "                condition: sel", false));
+        });
+    }
+
     private OSQueryBackend testBackend() throws IOException {
         return new OSQueryBackend(testFieldMapping, false, true);
     }