diff --git a/its/ruling/src/test/resources/regex-examples/java-S5852.json b/its/ruling/src/test/resources/regex-examples/java-S5852.json index 724ad1718bf..9226372113b 100644 --- a/its/ruling/src/test/resources/regex-examples/java-S5852.json +++ b/its/ruling/src/test/resources/regex-examples/java-S5852.json @@ -115,8 +115,6 @@ 473, 475, 479, -511, -529, 551, 557, 717, @@ -142,7 +140,6 @@ 1559, 1673, 1741, -1801, 1871, 1897, 1899, @@ -612,7 +609,6 @@ 1509, 1523, 1771, -1795, 1805, 1807, 1817, @@ -632,7 +628,6 @@ 225, 227, 231, -261, 277, 331, 347, @@ -643,7 +638,6 @@ 633, 637, 645, -649, 657, 659, 665, @@ -654,7 +648,6 @@ 697, 699, 715, -729, 791, 861, 869, @@ -664,9 +657,6 @@ 929, 933, 935, -945, -947, -949, 951, 953, 965, diff --git a/java-checks-test-sources/src/main/java/checks/regex/RedosCheck.java b/java-checks-test-sources/src/main/java/checks/regex/RedosCheck.java index 8314ff3ac3c..9f10e971c68 100644 --- a/java-checks-test-sources/src/main/java/checks/regex/RedosCheck.java +++ b/java-checks-test-sources/src/main/java/checks/regex/RedosCheck.java @@ -56,7 +56,9 @@ void alwaysQuadratic(String str) { str.matches("x*a*x*"); // Noncompliant {{Make sure the regex used here, which is vulnerable to polynomial runtime due to backtracking, cannot lead to denial of service.}} str.matches("x*,a*x*"); // Compliant, can fail between the two quantifiers str.matches("x*(xy?)*"); // Noncompliant {{Make sure the regex used here, which is vulnerable to polynomial runtime due to backtracking, cannot lead to denial of service.}} - str.matches("(ab)*a(ba)*"); // Noncompliant {{Make sure the regex used here, which is vulnerable to polynomial runtime due to backtracking, cannot lead to denial of service.}} + str.matches("(ab)*a(ba)*"); // False Negative :-( + str.matches("x*xx*"); // Noncompliant {{Make sure the regex used here, which is vulnerable to polynomial runtime due to backtracking, cannot lead to denial of service.}} + str.matches("x*yx*"); // Compliant str.matches("x*a*b*c*d*e*f*g*h*i*x*"); // Noncompliant {{Make sure the regex used here, which is vulnerable to polynomial runtime due to backtracking, cannot lead to denial of service.}} str.matches("x*a*b*c*d*e*f*g*h*i*j*x*"); // FN because we forget about the first x* when the maximum number of tracked repetitions is exceeded str.matches("x*a*b*c*d*e*f*g*h*i*j*x*x*"); // Noncompliant {{Make sure the regex used here, which is vulnerable to polynomial runtime due to backtracking, cannot lead to denial of service.}} @@ -125,8 +127,8 @@ void compliant(String str) { str.matches("(;*,)*"); str.matches("x*|x*"); str.matches("a*b*"); - str.matches("a*a?b*"); // Noncompliant - false positive :-( - str.matches("a*(a?b)*"); // Noncompliant - false positive :-( + str.matches("a*a?b*"); + str.matches("a*(a?b)*"); str.matches("a*(ab)*"); str.split("x*x*"); str.matches("(?s)x*.*"); diff --git a/java-checks/src/main/java/org/sonar/java/checks/regex/RedosCheck.java b/java-checks/src/main/java/org/sonar/java/checks/regex/RedosCheck.java index 7a092c816db..d4d0be96fb0 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/regex/RedosCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/regex/RedosCheck.java @@ -179,9 +179,12 @@ private void checkForOverlappingRepetitions(RepetitionTree tree) { if (tree.getQuantifier().isOpenEnded() && canFail(tree)) { for (RepetitionTree repetition : nonPossessiveRepetitions) { if (reachabilityChecker.canReach(repetition, tree)) { - SubAutomaton auto1 = new SubAutomaton(repetition.getElement(), tree.continuation(), false); - SubAutomaton auto2 = new SubAutomaton(repetition.continuation(), tree.continuation(), false); - if (intersectionChecker.check(auto1, auto2)) { + SubAutomaton repetitionAuto = new SubAutomaton(repetition.getElement(), repetition.continuation(), false); + SubAutomaton continuationAuto = new SubAutomaton(repetition.continuation(), tree, false); + SubAutomaton treeAuto = new SubAutomaton(tree.getElement(), tree.continuation(), false); + if (subAutomatonCanConsume(repetitionAuto, continuationAuto) + && automatonIsEmptyOrIntersects(continuationAuto, treeAuto) + && intersectionChecker.check(repetitionAuto, treeAuto)) { addBacktracking(BacktrackingType.ALWAYS_QUADRATIC); } } @@ -193,6 +196,16 @@ private void checkForOverlappingRepetitions(RepetitionTree tree) { } } + private boolean subAutomatonCanConsume(SubAutomaton auto1, SubAutomaton auto2) { + return canReachWithoutConsumingInputOrGoingThroughBoundaries(auto1.end, auto2.end) + || intersectionChecker.check(auto1, auto2); + } + + private boolean automatonIsEmptyOrIntersects(SubAutomaton auto1, SubAutomaton auto2) { + return canReachWithoutConsumingInputOrGoingThroughBoundaries(auto1.start, auto1.end) + || intersectionChecker.check(auto1, auto2); + } + private void addIfNonPossessive(RepetitionTree tree) { if (!tree.isPossessive()) { nonPossessiveRepetitions.add(tree);