Merge pull request #2334 from malice00/fix/2144

Changed purl-policy validation to use 'matches()' instead of 'contains(), so RegExes can be used
DependencyTrack · Jan 28, 2023 · 0ce7709 · 0ce7709
2 parents 99dc805 + e6b476c
commit 0ce7709
Show file tree

Hide file tree

Showing 7 changed files with 216 additions and 22 deletions.
diff --git a/src/main/java/org/dependencytrack/policy/CoordinatesPolicyEvaluator.java b/src/main/java/org/dependencytrack/policy/CoordinatesPolicyEvaluator.java
@@ -74,22 +74,11 @@ private boolean matches(final PolicyCondition.Operator operator, final String co
             return true;
         }
         final String p = StringUtils.trimToNull(part);
-        if (PolicyCondition.Operator.MATCHES == operator) {
-            if (p != null) {
-                if ("*".equals(conditionValue)) {
-                    return true;
-                } else if (conditionValue != null && p.contains(conditionValue)) {
-                    return true;
-                }
-            }
-        } else if (PolicyCondition.Operator.NO_MATCH == operator) {
-            if (p != null) {
-                if ("*".equals(conditionValue)) {
-                    return false;
-                } else if (conditionValue != null && p.contains(conditionValue)) {
-                    return false;
-                }
-                return true;
+        if (p != null) {
+            if (PolicyCondition.Operator.MATCHES == operator) {
+                return org.dependencytrack.policy.Matcher.matches(p, conditionValue);
+            } else if (PolicyCondition.Operator.NO_MATCH == operator) {
+                return !org.dependencytrack.policy.Matcher.matches(p, conditionValue);
             }
         }
         return false;

diff --git a/src/main/java/org/dependencytrack/policy/CpePolicyEvaluator.java b/src/main/java/org/dependencytrack/policy/CpePolicyEvaluator.java
@@ -56,11 +56,11 @@ public List<PolicyConditionViolation> evaluate(final Policy policy, final Compon
         for (final PolicyCondition condition: super.extractSupportedConditions(policy)) {
             LOGGER.debug("Evaluating component (" + component.getUuid() + ") against policy condition (" + condition.getUuid() + ")");
             if (PolicyCondition.Operator.MATCHES == condition.getOperator()) {
-                if (component.getCpe() != null && component.getCpe().contains(condition.getValue())) {
+                if (Matcher.matches(component.getCpe(), condition.getValue())) {
                     violations.add(new PolicyConditionViolation(condition, component));
                 }
             } else if (PolicyCondition.Operator.NO_MATCH == condition.getOperator()) {
-                if (component.getCpe() != null && !component.getCpe().contains(condition.getValue())) {
+                if (!Matcher.matches(component.getCpe(), condition.getValue())) {
                     violations.add(new PolicyConditionViolation(condition, component));
                 }
             }

diff --git a/src/main/java/org/dependencytrack/policy/Matcher.java b/src/main/java/org/dependencytrack/policy/Matcher.java
@@ -0,0 +1,58 @@
+/*
+ * This file is part of Dependency-Track.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) Steve Springett. All Rights Reserved.
+ */
+package org.dependencytrack.policy;
+
+/**
+ * Reusable methods that PolicyEvaluator implementations can extend.
+ *
+ * @author Roland Asmann
+ * @since 4.8.0
+ */
+public final class Matcher {
+
+    private Matcher() {
+      // Utility-class should not be instantiated
+    }
+
+    /**
+     * Check if the given value matches with the conditionString. If the
+     * conditionString is not a regular expression, turn it into one.
+     * 
+     * @param value           The value to match against
+     * @param conditionString The condition that should match -- may or may not be a
+     *                        regular expression
+     * @return <code>true</code> if the value matches the conditionString
+     */
+    static boolean matches(String value, String conditionString) {
+        if (value == null && conditionString == null) {
+            return true;
+        }
+        if (value == null ^ conditionString == null) {
+            return false;
+        }
+        conditionString = conditionString.replace("*", ".*").replace("..*", ".*");
+        if (!conditionString.startsWith("^") && !conditionString.startsWith(".*")) {
+            conditionString = ".*" + conditionString;
+        }
+        if (!conditionString.endsWith("$") && !conditionString.endsWith(".*")) {
+            conditionString += ".*";
+        }
+        return value.matches(conditionString);
+    }
+}
diff --git a/src/main/java/org/dependencytrack/policy/PackageURLPolicyEvaluator.java b/src/main/java/org/dependencytrack/policy/PackageURLPolicyEvaluator.java
@@ -56,11 +56,11 @@ public List<PolicyConditionViolation> evaluate(final Policy policy, final Compon
         for (final PolicyCondition condition: super.extractSupportedConditions(policy)) {
             LOGGER.debug("Evaluating component (" + component.getUuid() + ") against policy condition (" + condition.getUuid() + ")");
             if (PolicyCondition.Operator.MATCHES == condition.getOperator()) {
-                if (component.getPurl().canonicalize().contains(condition.getValue())) {
+                if (Matcher.matches(component.getPurl().canonicalize(), condition.getValue())) {
                     violations.add(new PolicyConditionViolation(condition, component));
                 }
             } else if (PolicyCondition.Operator.NO_MATCH == condition.getOperator()) {
-                if (!component.getPurl().canonicalize().contains(condition.getValue())) {
+                if (!Matcher.matches(component.getPurl().canonicalize(), condition.getValue())) {
                     violations.add(new PolicyConditionViolation(condition, component));
                 }
             }

diff --git a/src/main/java/org/dependencytrack/policy/SwidTagIdPolicyEvaluator.java b/src/main/java/org/dependencytrack/policy/SwidTagIdPolicyEvaluator.java
@@ -56,11 +56,11 @@ public List<PolicyConditionViolation> evaluate(final Policy policy, final Compon
         for (final PolicyCondition condition: super.extractSupportedConditions(policy)) {
             LOGGER.debug("Evaluating component (" + component.getUuid() + ") against policy condition (" + condition.getUuid() + ")");
             if (PolicyCondition.Operator.MATCHES == condition.getOperator()) {
-                if (component.getSwidTagId() != null && component.getSwidTagId().contains(condition.getValue())) {
+                if (Matcher.matches(component.getSwidTagId(), condition.getValue())) {
                     violations.add(new PolicyConditionViolation(condition, component));
                 }
             } else if (PolicyCondition.Operator.NO_MATCH == condition.getOperator()) {
-                if (component.getSwidTagId() != null && !component.getSwidTagId().contains(condition.getValue())) {
+                if (!Matcher.matches(component.getSwidTagId(), condition.getValue())) {
                     violations.add(new PolicyConditionViolation(condition, component));
                 }
             }

diff --git a/src/test/java/org/dependencytrack/policy/MatcherTest.java b/src/test/java/org/dependencytrack/policy/MatcherTest.java
@@ -0,0 +1,57 @@
+/*
+ * This file is part of Dependency-Track.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) Steve Springett. All Rights Reserved.
+ */
+package org.dependencytrack.policy;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class MatcherTest {
+
+    @Test
+    public void checkNulls() {
+        Assert.assertTrue(Matcher.matches(null, null));
+        Assert.assertFalse(Matcher.matches("", null));
+        Assert.assertFalse(Matcher.matches(null, ""));
+    }
+
+    @Test
+    public void checkExact() {
+        Assert.assertTrue(Matcher.matches("", ""));
+        Assert.assertTrue(Matcher.matches("something", "something"));
+    }
+
+    @Test
+    public void checkPartials() {
+        Assert.assertTrue(Matcher.matches("something", "some"));
+        Assert.assertTrue(Matcher.matches("something", "meth"));
+        Assert.assertTrue(Matcher.matches("something", "thing"));
+    }
+
+    @Test
+    public void checkWildcards() {
+        Assert.assertTrue(Matcher.matches("something", "some*"));
+        Assert.assertTrue(Matcher.matches("something", "*thing"));
+    }
+
+    @Test
+    public void checkRegex() {
+        Assert.assertTrue(Matcher.matches("something", "^some.*"));
+        Assert.assertTrue(Matcher.matches("something", ".*thing$"));
+    }
+}
diff --git a/src/test/java/org/dependencytrack/policy/PackageURLPolicyEvaluatorTest.java b/src/test/java/org/dependencytrack/policy/PackageURLPolicyEvaluatorTest.java
@@ -103,4 +103,94 @@ public void issue1925_no_match() throws Exception {
         List<PolicyConditionViolation> violations = evaluator.evaluate(policy, component);
         Assert.assertEquals(0, violations.size());
     }
+
+    @Test
+    public void issue2144_existing1() throws Exception {
+        Policy policy = qm.createPolicy("Test Policy", Policy.Operator.ANY, Policy.ViolationState.INFO);
+        PolicyCondition condition = qm.createPolicyCondition(policy, PolicyCondition.Subject.PACKAGE_URL, PolicyCondition.Operator.MATCHES, "pkg:generic/com/acme/[email protected]");
+        Component component = new Component();
+        component.setPurl(new PackageURL("pkg:generic/com/acme/[email protected]?type=jar"));
+        component.setPurlCoordinates(new PackageURL("pkg:generic/com/acme/[email protected]"));
+        PolicyEvaluator evaluator = new PackageURLPolicyEvaluator();
+        List<PolicyConditionViolation> violations = evaluator.evaluate(policy, component);
+        Assert.assertEquals(1, violations.size());
+        PolicyConditionViolation violation = violations.get(0);
+        Assert.assertEquals(component, violation.getComponent());
+        Assert.assertEquals(condition, violation.getPolicyCondition());
+    }
+
+    @Test
+    public void issue2144_existing2() throws Exception {
+        Policy policy = qm.createPolicy("Test Policy", Policy.Operator.ANY, Policy.ViolationState.INFO);
+        PolicyCondition condition = qm.createPolicyCondition(policy, PolicyCondition.Subject.PACKAGE_URL, PolicyCondition.Operator.MATCHES, "/com/acme/");
+        Component component = new Component();
+        component.setPurl(new PackageURL("pkg:generic/com/acme/[email protected]?type=jar"));
+        component.setPurlCoordinates(new PackageURL("pkg:generic/com/acme/[email protected]"));
+        PolicyEvaluator evaluator = new PackageURLPolicyEvaluator();
+        List<PolicyConditionViolation> violations = evaluator.evaluate(policy, component);
+        Assert.assertEquals(1, violations.size());
+        PolicyConditionViolation violation = violations.get(0);
+        Assert.assertEquals(component, violation.getComponent());
+        Assert.assertEquals(condition, violation.getPolicyCondition());
+    }
+
+    @Test
+    public void issue2144_groupIdWithDotMatchesSlash() throws Exception {
+        Policy policy = qm.createPolicy("Test Policy", Policy.Operator.ANY, Policy.ViolationState.INFO);
+        PolicyCondition condition = qm.createPolicyCondition(policy, PolicyCondition.Subject.PACKAGE_URL, PolicyCondition.Operator.MATCHES, "/com.acme/");
+        Component component = new Component();
+        component.setPurl(new PackageURL("pkg:generic/com/acme/[email protected]?type=jar"));
+        component.setPurlCoordinates(new PackageURL("pkg:generic/com/acme/[email protected]"));
+        PolicyEvaluator evaluator = new PackageURLPolicyEvaluator();
+        List<PolicyConditionViolation> violations = evaluator.evaluate(policy, component);
+        Assert.assertEquals(1, violations.size());
+        PolicyConditionViolation violation = violations.get(0);
+        Assert.assertEquals(component, violation.getComponent());
+        Assert.assertEquals(condition, violation.getPolicyCondition());
+    }
+
+    @Test
+    public void issue2144_wildcard() throws Exception {
+        Policy policy = qm.createPolicy("Test Policy", Policy.Operator.ANY, Policy.ViolationState.INFO);
+        PolicyCondition condition = qm.createPolicyCondition(policy, PolicyCondition.Subject.PACKAGE_URL, PolicyCondition.Operator.MATCHES, ".*com.acme.*");
+        Component component = new Component();
+        component.setPurl(new PackageURL("pkg:generic/com/acme/[email protected]?type=jar"));
+        component.setPurlCoordinates(new PackageURL("pkg:generic/com/acme/[email protected]"));
+        PolicyEvaluator evaluator = new PackageURLPolicyEvaluator();
+        List<PolicyConditionViolation> violations = evaluator.evaluate(policy, component);
+        Assert.assertEquals(1, violations.size());
+        PolicyConditionViolation violation = violations.get(0);
+        Assert.assertEquals(component, violation.getComponent());
+        Assert.assertEquals(condition, violation.getPolicyCondition());
+    }
+
+    @Test
+    public void issue2144_wildcard2() throws Exception {
+        Policy policy = qm.createPolicy("Test Policy", Policy.Operator.ANY, Policy.ViolationState.INFO);
+        PolicyCondition condition = qm.createPolicyCondition(policy, PolicyCondition.Subject.PACKAGE_URL, PolicyCondition.Operator.MATCHES, ".*acme.*myCompany.*");
+        Component component = new Component();
+        component.setPurl(new PackageURL("pkg:generic/com/acme/[email protected]?type=jar"));
+        component.setPurlCoordinates(new PackageURL("pkg:generic/com/acme/[email protected]"));
+        PolicyEvaluator evaluator = new PackageURLPolicyEvaluator();
+        List<PolicyConditionViolation> violations = evaluator.evaluate(policy, component);
+        Assert.assertEquals(1, violations.size());
+        PolicyConditionViolation violation = violations.get(0);
+        Assert.assertEquals(component, violation.getComponent());
+        Assert.assertEquals(condition, violation.getPolicyCondition());
+    }
+
+    @Test
+    public void issue2144_wildcard3() throws Exception {
+        Policy policy = qm.createPolicy("Test Policy", Policy.Operator.ANY, Policy.ViolationState.INFO);
+        PolicyCondition condition = qm.createPolicyCondition(policy, PolicyCondition.Subject.PACKAGE_URL, PolicyCondition.Operator.MATCHES, ".*(a|b|c)cme.*");
+        Component component = new Component();
+        component.setPurl(new PackageURL("pkg:generic/com/acme/[email protected]?type=jar"));
+        component.setPurlCoordinates(new PackageURL("pkg:generic/com/acme/[email protected]"));
+        PolicyEvaluator evaluator = new PackageURLPolicyEvaluator();
+        List<PolicyConditionViolation> violations = evaluator.evaluate(policy, component);
+        Assert.assertEquals(1, violations.size());
+        PolicyConditionViolation violation = violations.get(0);
+        Assert.assertEquals(component, violation.getComponent());
+        Assert.assertEquals(condition, violation.getPolicyCondition());
+    }
 }