Ruby: regex: fix getGroupNumber

non-capture groups should not have a group number

Author: aibaars

ruby/ql/lib/codeql/ruby/security/performance/ParseRegExp.qll

@@ -480,6 +480,7 @@ abstract class RegExp extends AST::StringlikeLiteral {
   /** Gets the number of the group in start,end */
   int getGroupNumber(int start, int end) {
     this.group(start, end) and
+    not this.nonCapturingGroupStart(start, _) and
     result =
       count(int i | this.group(i, _) and i < start and not this.nonCapturingGroupStart(i, _)) + 1
   }
@@ -583,7 +584,7 @@ abstract class RegExp extends AST::StringlikeLiteral {
   private predicate nonCapturingGroupStart(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
-    this.getChar(start + 2) = ":" and
+    this.getChar(start + 2) = [":", "=", "<", "!", "#"] and
     end = start + 3
   }
 

ruby/ql/test/library-tests/regexp/regexp.expected

@@ -6,11 +6,8 @@ groupNumber
 | regexp.rb:46:2:46:6 | (foo) | 1 |
 | regexp.rb:47:4:47:8 | (o\|b) | 1 |
 | regexp.rb:48:2:48:9 | (a\|b\|cd) | 1 |
-| regexp.rb:49:2:49:7 | (?::+) | 1 |
-| regexp.rb:52:2:52:11 | (?<id>\\w+) | 1 |
 | regexp.rb:53:2:53:12 | (?'foo'fo+) | 1 |
 | regexp.rb:56:2:56:5 | (a+) | 1 |
-| regexp.rb:57:2:57:11 | (?<qux>q+) | 1 |
 term
 | regexp.rb:5:2:5:4 | abc | RegExpConstant,RegExpNormalChar |
 | regexp.rb:8:2:8:2 | a | RegExpConstant,RegExpNormalChar |

ruby/ql/test/query-tests/security/cwe-116/BadTagFilter.expected

@@ -11,4 +11,4 @@
 | test.rb:15:6:15:39 | <script[^>]*?>[\\s\\S]*?<\\/script.*> | This regular expression does not match script end tags like </script\\t\\n bar>. |
 | test.rb:17:6:17:40 | <script\\b[^>]*>([\\s\\S]*?)<\\/script> | This regular expression does not match script end tags like </script >. |
 | test.rb:18:6:18:48 | <(?:!--([\\S\|\\s]*?)-->)\|([^\\/\\s>]+)[\\S\\s]*?> | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 1 and comments ending with --!> are matched with capture group 2. |
-| test.rb:19:6:19:147 | <(?:(?:\\/([^>]+)>)\|(?:!--([\\S\|\\s]*?)-->)\|(?:([^\\/\\s>]+)((?:\\s+[\\w\\-:.]+(?:\\s*=\\s*?(?:(?:"[^"]*")\|(?:'[^']*')\|[^\\s"'\\/>]+))?)*)[\\S\\s]*?(\\/?)>)) | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 2 and comments ending with --!> are matched with capture group 1, 3, 4, 5. |
+| test.rb:19:6:19:147 | <(?:(?:\\/([^>]+)>)\|(?:!--([\\S\|\\s]*?)-->)\|(?:([^\\/\\s>]+)((?:\\s+[\\w\\-:.]+(?:\\s*=\\s*?(?:(?:"[^"]*")\|(?:'[^']*')\|[^\\s"'\\/>]+))?)*)[\\S\\s]*?(\\/?)>)) | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 2 and comments ending with --!> are matched with capture group 3, 4. |