Merge pull request #8489 from aibaars/regex-refactor

Ruby: refactor regex libraries

Author: aibaars

python/ql/lib/semmle/python/RegexTreeView.qll

@@ -552,7 +552,7 @@ class RegExpWordBoundary extends RegExpSpecialChar {
 
 /**
  * A character class escape in a regular expression.
- * That is, an escaped charachter that denotes multiple characters.
+ * That is, an escaped character that denotes multiple characters.
  *
  * Examples:
  *

python/ql/lib/semmle/python/regex.qll

@@ -188,7 +188,7 @@ abstract class RegexString extends Expr {
     )
   }
 
-  /** Hold is a character set starts between `start` and `end`. */
+  /** Holds if a character set starts between `start` and `end`. */
   predicate char_set_start(int start, int end) {
     this.char_set_start(start) = true and
     (
@@ -316,8 +316,10 @@ abstract class RegexString extends Expr {
     result = this.(Bytes).getS()
   }
 
+  /** Gets the `i`th character of this regex */
   string getChar(int i) { result = this.getText().charAt(i) }
 
+  /** Gets the `i`th character of this regex, unless it is part of a character escape sequence. */
   string nonEscapedCharAt(int i) {
     result = this.getText().charAt(i) and
     not exists(int x, int y | this.escapedCharacter(x, y) and i in [x .. y - 1])
@@ -329,6 +331,9 @@ abstract class RegexString extends Expr {
 
   private predicate isGroupStart(int i) { this.nonEscapedCharAt(i) = "(" and not this.inCharSet(i) }
 
+  /**
+   * Holds if the `i`th character could not be parsed.
+   */
   predicate failedToParse(int i) {
     exists(this.getChar(i)) and
     not exists(int start, int end |
@@ -417,6 +422,9 @@ abstract class RegexString extends Expr {
     )
   }
 
+  /**
+   * Holds if a simple or escaped character is found between `start` and `end`.
+   */
   predicate character(int start, int end) {
     (
       this.simpleCharacter(start, end) and
@@ -428,12 +436,18 @@ abstract class RegexString extends Expr {
     not exists(int x, int y | this.backreference(x, y) and x <= start and y >= end)
   }
 
+  /**
+   * Holds if a normal character is found between `start` and `end`.
+   */
   predicate normalCharacter(int start, int end) {
     end = start + 1 and
     this.character(start, end) and
     not this.specialCharacter(start, end, _)
   }
 
+  /**
+   * Holds if a special character is found between `start` and `end`.
+   */
   predicate specialCharacter(int start, int end, string char) {
     not this.inCharSet(start) and
     this.character(start, end) and
@@ -492,7 +506,7 @@ abstract class RegexString extends Expr {
     this.specialCharacter(start, end, _)
   }
 
-  /** Whether the text in the range start,end is a group */
+  /** Whether the text in the range `start,end` is a group */
   predicate group(int start, int end) {
     this.groupContents(start, end, _, _)
     or
@@ -611,19 +625,26 @@ abstract class RegexString extends Expr {
     this.simple_group_start(start, end)
   }
 
+  /** Matches the start of a non-capturing group, e.g. `(?:` */
   private predicate non_capturing_group_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
     this.getChar(start + 2) = ":" and
     end = start + 3
   }
 
+  /** Matches the start of a simple group, e.g. `(a+)`. */
   private predicate simple_group_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) != "?" and
     end = start + 1
   }
 
+  /**
+   * Matches the start of a named group, such as:
+   * - `(?<name>\w+)`
+   * - `(?'name'\w+)`
+   */
   private predicate named_group_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
@@ -675,20 +696,23 @@ abstract class RegexString extends Expr {
     )
   }
 
+  /** Matches the start of a positive lookahead assertion, i.e. `(?=`. */
   private predicate lookahead_assertion_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
     this.getChar(start + 2) = "=" and
     end = start + 3
   }
 
+  /** Matches the start of a negative lookahead assertion, i.e. `(?!`. */
   private predicate negative_lookahead_assertion_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
     this.getChar(start + 2) = "!" and
     end = start + 3
   }
 
+  /** Matches the start of a positive lookbehind assertion, i.e. `(?<=`. */
   private predicate lookbehind_assertion_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
@@ -697,6 +721,7 @@ abstract class RegexString extends Expr {
     end = start + 4
   }
 
+  /** Matches the start of a negative lookbehind assertion, i.e. `(?<!`. */
   private predicate negative_lookbehind_assertion_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
@@ -705,26 +730,30 @@ abstract class RegexString extends Expr {
     end = start + 4
   }
 
+  /** Matches the start of a comment group, i.e. `(?#`. */
   private predicate comment_group_start(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
     this.getChar(start + 2) = "#" and
     end = start + 3
   }
 
+  /** Matches the contents of a group. */
   predicate groupContents(int start, int end, int in_start, int in_end) {
     this.group_start(start, in_start) and
     end = in_end + 1 and
     this.top_level(in_start, in_end) and
     this.isGroupEnd(in_end)
   }
 
+  /** Matches a named backreference, e.g. `\k<foo>`. */
   private predicate named_backreference(int start, int end, string name) {
     this.named_backreference_start(start, start + 4) and
     end = min(int i | i > start + 4 and this.getChar(i) = ")") + 1 and
     name = this.getText().substring(start + 4, end - 2)
   }
 
+  /** Matches a numbered backreference, e.g. `\1`. */
   private predicate numbered_backreference(int start, int end, int value) {
     this.escapingChar(start) and
     // starting with 0 makes it an octal escape
@@ -749,7 +778,7 @@ abstract class RegexString extends Expr {
     )
   }
 
-  /** Whether the text in the range start,end is a back reference */
+  /** Whether the text in the range `start,end` is a back reference */
   predicate backreference(int start, int end) {
     this.numbered_backreference(start, end, _)
     or

ruby/ql/consistency-queries/RegExpConsistency.ql

@@ -1,4 +1,4 @@
-import codeql.ruby.security.performance.RegExpTreeView
+import codeql.ruby.Regexp
 
 query predicate nonUniqueChild(RegExpParent parent, int i, RegExpTerm child) {
   child = parent.getChild(i) and

ruby/ql/lib/change-notes/2022-03-21-regex.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `ParseRegExp` and `RegExpTreeView` modules are now "internal" modules. Users should use `codeql.ruby.Regexp` instead.

ruby/ql/lib/codeql/ruby/Regexp.qll

@@ -0,0 +1,143 @@
+/**
+ * Provides classes for working with regular expressions.
+ *
+ * Regular expression literals are represented as an abstract syntax tree of regular expression
+ * terms.
+ */
+
+import regexp.RegExpTreeView // re-export
+private import regexp.internal.ParseRegExp
+private import codeql.ruby.ast.Literal as AST
+private import codeql.ruby.DataFlow
+private import codeql.ruby.controlflow.CfgNodes
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.dataflow.internal.tainttrackingforlibraries.TaintTrackingImpl
+
+/**
+ * Provides utility predicates related to regular expressions.
+ */
+module RegExpPatterns {
+  /**
+   * Gets a pattern that matches common top-level domain names in lower case.
+   */
+  string getACommonTld() {
+    // according to ranking by http://google.com/search?q=site:.<<TLD>>
+    result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
+  }
+}
+
+/**
+ * A node whose value may flow to a position where it is interpreted
+ * as a part of a regular expression.
+ */
+abstract class RegExpPatternSource extends DataFlow::Node {
+  /**
+   * Gets a node where the pattern of this node is parsed as a part of
+   * a regular expression.
+   */
+  abstract DataFlow::Node getAParse();
+
+  /**
+   * Gets the root term of the regular expression parsed from this pattern.
+   */
+  abstract RegExpTerm getRegExpTerm();
+}
+
+/**
+ * A regular expression literal, viewed as the pattern source for itself.
+ */
+private class RegExpLiteralPatternSource extends RegExpPatternSource {
+  private AST::RegExpLiteral astNode;
+
+  RegExpLiteralPatternSource() { astNode = this.asExpr().getExpr() }
+
+  override DataFlow::Node getAParse() { result = this }
+
+  override RegExpTerm getRegExpTerm() { result = astNode.getParsed() }
+}
+
+/**
+ * A node whose string value may flow to a position where it is interpreted
+ * as a part of a regular expression.
+ */
+private class StringRegExpPatternSource extends RegExpPatternSource {
+  private DataFlow::Node parse;
+
+  StringRegExpPatternSource() { this = regExpSource(parse) }
+
+  override DataFlow::Node getAParse() { result = parse }
+
+  override RegExpTerm getRegExpTerm() { result.getRegExp() = this.asExpr().getExpr() }
+}
+
+private class RegExpLiteralRegExp extends RegExp, AST::RegExpLiteral {
+  override predicate isDotAll() { this.hasMultilineFlag() }
+
+  override predicate isIgnoreCase() { this.hasCaseInsensitiveFlag() }
+
+  override string getFlags() { result = this.getFlagString() }
+}
+
+private class ParsedStringRegExp extends RegExp {
+  private DataFlow::Node parse;
+
+  ParsedStringRegExp() { this = regExpSource(parse).asExpr().getExpr() }
+
+  DataFlow::Node getAParse() { result = parse }
+
+  override predicate isDotAll() { none() }
+
+  override predicate isIgnoreCase() { none() }
+
+  override string getFlags() { none() }
+}
+
+/**
+ * Holds if `source` may be interpreted as a regular expression.
+ */
+private predicate isInterpretedAsRegExp(DataFlow::Node source) {
+  // The first argument to an invocation of `Regexp.new` or `Regexp.compile`.
+  source = API::getTopLevelMember("Regexp").getAMethodCall(["compile", "new"]).getArgument(0)
+  or
+  // The argument of a call that coerces the argument to a regular expression.
+  exists(DataFlow::CallNode mce |
+    mce.getMethodName() = ["match", "match?"] and
+    source = mce.getArgument(0) and
+    // exclude https://ruby-doc.org/core-2.4.0/Regexp.html#method-i-match
+    not mce.getReceiver().asExpr().getExpr() instanceof AST::RegExpLiteral
+  )
+}
+
+private class RegExpConfiguration extends Configuration {
+  RegExpConfiguration() { this = "RegExpConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr() =
+      any(ExprCfgNode e |
+        e.getConstantValue().isString(_) and
+        not e instanceof ExprNodes::VariableReadAccessCfgNode and
+        not e instanceof ExprNodes::ConstantReadAccessCfgNode
+      )
+  }
+
+  override predicate isSink(DataFlow::Node sink) { isInterpretedAsRegExp(sink) }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    // stop flow if `node` is receiver of
+    // https://ruby-doc.org/core-2.4.0/String.html#method-i-match
+    exists(DataFlow::CallNode mce |
+      mce.getMethodName() = ["match", "match?"] and
+      node = mce.getReceiver() and
+      mce.getArgument(0).asExpr().getExpr() instanceof AST::RegExpLiteral
+    )
+  }
+}
+
+/**
+ * Gets a node whose value may flow (inter-procedurally) to `re`, where it is interpreted
+ * as a part of a regular expression.
+ */
+cached
+DataFlow::Node regExpSource(DataFlow::Node re) {
+  exists(RegExpConfiguration c | c.hasFlow(result, re))
+}

ruby/ql/lib/codeql/ruby/ast/Literal.qll

@@ -1,5 +1,5 @@
 private import codeql.ruby.AST
-private import codeql.ruby.security.performance.RegExpTreeView as RETV
+private import codeql.ruby.Regexp as RE
 private import internal.AST
 private import internal.Constant
 private import internal.Literal
@@ -393,7 +393,7 @@ class RegExpLiteral extends StringlikeLiteral instanceof RegExpLiteralImpl {
   final predicate hasFreeSpacingFlag() { this.getFlagString().charAt(_) = "x" }
 
   /** Returns the root node of the parse tree of this regular expression. */
-  final RETV::RegExpTerm getParsed() { result = RETV::getParsedRegExp(this) }
+  final RE::RegExpTerm getParsed() { result = RE::getParsedRegExp(this) }
 }
 
 /**

ruby/ql/lib/codeql/ruby/printAst.qll

@@ -7,7 +7,7 @@
  */
 
 private import AST
-private import codeql.ruby.security.performance.RegExpTreeView as RETV
+private import codeql.ruby.Regexp as RE
 private import codeql.ruby.ast.internal.Synthesis
 
 /**
@@ -37,7 +37,7 @@ private predicate shouldPrintAstEdge(AstNode parent, string edgeName, AstNode ch
 
 newtype TPrintNode =
   TPrintRegularAstNode(AstNode n) { shouldPrintNode(n) } or
-  TPrintRegExpNode(RETV::RegExpTerm term) {
+  TPrintRegExpNode(RE::RegExpTerm term) {
     exists(RegExpLiteral literal |
       shouldPrintNode(literal) and
       term.getRootTerm() = literal.getParsed()
@@ -107,7 +107,7 @@ class PrintRegularAstNode extends PrintAstNode, TPrintRegularAstNode {
     or
     // If this AST node is a regexp literal, add the parsed regexp tree as a
     // child.
-    exists(RETV::RegExpTerm t | t = astNode.(RegExpLiteral).getParsed() |
+    exists(RE::RegExpTerm t | t = astNode.(RegExpLiteral).getParsed() |
       result = TPrintRegExpNode(t) and edgeName = "getParsed"
     )
   }
@@ -134,7 +134,7 @@ class PrintRegularAstNode extends PrintAstNode, TPrintRegularAstNode {
 
 /** A parsed regexp node in the output tree. */
 class PrintRegExpNode extends PrintAstNode, TPrintRegExpNode {
-  RETV::RegExpTerm regexNode;
+  RE::RegExpTerm regexNode;
 
   PrintRegExpNode() { this = TPrintRegExpNode(regexNode) }
 
@@ -147,7 +147,7 @@ class PrintRegExpNode extends PrintAstNode, TPrintRegExpNode {
     exists(int i | result = TPrintRegExpNode(regexNode.getChild(i)) and edgeName = i.toString())
   }
 
-  override int getOrder() { exists(RETV::RegExpTerm p | p.getChild(result) = regexNode) }
+  override int getOrder() { exists(RE::RegExpTerm p | p.getChild(result) = regexNode) }
 
   override predicate hasLocationInfo(
     string filepath, int startline, int startcolumn, int endline, int endcolumn