Merge pull request #292 from github/regexp_slash_az

Don't parse `\A` and `\Z` as `RegExpConstant`

Author: nickrolfe

ql/lib/codeql/ruby/regexp/ParseRegExp.qll

@@ -388,10 +388,17 @@ class RegExp extends AST::RegExpLiteral {
 
   predicate specialCharacter(int start, int end, string char) {
     this.character(start, end) and
-    end = start + 1 and
-    char = this.getChar(start) and
-    (char = "$" or char = "^" or char = ".") and
-    not this.inCharSet(start)
+    not this.inCharSet(start) and
+    (
+      end = start + 1 and
+      char = this.getChar(start) and
+      (char = "$" or char = "^" or char = ".")
+      or
+      end = start + 2 and
+      this.escapingChar(start) and
+      char = this.getText().substring(start, end) and
+      char = ["\\A", "\\Z", "\\z"]
+    )
   }
 
   /** Whether the text in the range `start,end` is a group */
@@ -808,10 +815,8 @@ class RegExp extends AST::RegExpLiteral {
       or
       this.qualifiedItem(x, start, true, _)
       or
-      this.specialCharacter(x, start, "^")
-      or
-      // \A matches the start of the string
-      x = start - 2 and this.escapingChar(x) and this.getChar(x + 1) = "A"
+      // ^ and \A match the start of the string
+      this.specialCharacter(x, start, ["^", "\\A"])
     )
     or
     exists(int y | this.firstPart(start, y) |
@@ -836,9 +841,8 @@ class RegExp extends AST::RegExpLiteral {
       or
       this.qualifiedItem(end, y, true, _)
       or
-      this.specialCharacter(end, y, "$")
-      or
-      y = end + 2 and this.escapingChar(end) and this.getChar(end + 1) = "Z"
+      // $, \Z, and \z match the end of the string.
+      this.specialCharacter(end, y, ["$", "\\Z", "\\z"])
     )
     or
     exists(int x |

ql/lib/codeql/ruby/regexp/RegExpTreeView.qll

@@ -594,13 +594,13 @@ class RegExpDot extends RegExpSpecialChar {
 }
 
 class RegExpDollar extends RegExpSpecialChar {
-  RegExpDollar() { this.getChar() = "$" }
+  RegExpDollar() { this.getChar() = ["$", "\\Z", "\\z"] }
 
   override string getAPrimaryQlClass() { result = "RegExpDollar" }
 }
 
 class RegExpCaret extends RegExpSpecialChar {
-  RegExpCaret() { this.getChar() = "^" }
+  RegExpCaret() { this.getChar() = ["^", "\\A"] }
 
   override string getAPrimaryQlClass() { result = "RegExpCaret" }
 }

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

@@ -146,10 +146,10 @@ regexp.rb:
 
 #   19| [RegExpConstant, RegExpNormalChar] _
 
-#   20| [RegExpConstant, RegExpEscape] \A
+#   20| [RegExpCaret] \A
 
 #   20| [RegExpSequence] \A[+-]?\d+
-#-----| 0 -> [RegExpConstant, RegExpEscape] \A
+#-----| 0 -> [RegExpCaret] \A
 #-----| 1 -> [RegExpOpt] [+-]?
 #-----| 2 -> [RegExpPlus] \d+
 

ql/test/query-tests/security/cwe-1333-polynomial-redos/PolynomialReDoS.rb

@@ -33,7 +33,10 @@ def some_request_handler
 
     # GOOD - guarded by a string length check
     if name.length < 1024
-	name.gsub regex, ''
+      name.gsub regex, ''
     end
+
+    # GOOD - regex does not suffer from polynomial backtracking (regression test)
+    params[:foo] =~ /\A[bc].*\Z/
   end
-end
+end