From a6264a7733a5996a226fa41647ee28bda337e405 Mon Sep 17 00:00:00 2001 From: Joanne Wang Date: Wed, 6 Mar 2024 05:34:20 -0800 Subject: [PATCH] Add throw for empty strings in rules with modifier contains, startwith, and endswith (#860) * add validation for empty strings with contains, startswith and endswith modifiers Signed-off-by: Joanne Wang * throw exception if empty string with contains, startswith, or endswith Signed-off-by: Joanne Wang * change var name Signed-off-by: Joanne Wang * add modifiers to log Signed-off-by: Joanne Wang --------- Signed-off-by: Joanne Wang (cherry picked from commit f4ee7bb9118a35ff4706a9e36abb9f2b042c069f) --- .../rules/objects/SigmaDetectionItem.java | 10 ++- .../rules/backend/QueryBackendTests.java | 72 +++++++++++++++++++ 2 files changed, 81 insertions(+), 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 index a334ca758..c74bd9177 100644 --- 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 SigmaDetectionItem fromMapping(String key, Either> List 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 index aff11d913..3f8196d3d 100644 --- 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); }