share {js,rb}/regex/missing-regexp-anchor

Author: erik-krogh

java/ql/lib/semmle/code/java/regex/RegexTreeView.qll

@@ -85,6 +85,9 @@ module Impl implements RegexTreeViewSig {
 
     /** Gets the associated regex. */
     abstract Regex getRegex();
+
+    /** Gets the last child term of this element. */
+    RegExpTerm getLastChild() { result = this.getChild(this.getNumChild() - 1) }
   }
 
   /**
@@ -567,6 +570,13 @@ module Impl implements RegexTreeViewSig {
     RegExpWordBoundary() { this.getChar() = "\\b" }
   }
 
+  /**
+   * A non-word boundary, that is, a regular expression term of the form `\B`.
+   */
+  class RegExpNonWordBoundary extends RegExpSpecialChar {
+    RegExpNonWordBoundary() { this.getChar() = "\\B" }
+  }
+
   /**
    * Gets the hex number for the `hex` char.
    */
@@ -922,6 +932,21 @@ module Impl implements RegexTreeViewSig {
     override string getPrimaryQLClass() { result = "RegExpDot" }
   }
 
+  /**
+   * A term that matches a specific position between characters in the string.
+   *
+   * Example:
+   *
+   * ```
+   * ^
+   * ```
+   */
+  class RegExpAnchor extends RegExpSpecialChar {
+    RegExpAnchor() { this.getChar() = ["$", "^"] }
+
+    override string getPrimaryQLClass() { result = "RegExpAnchor" }
+  }
+
   /**
    * A dollar assertion `$` matching the end of a line.
    *
@@ -931,7 +956,7 @@ module Impl implements RegexTreeViewSig {
    * $
    * ```
    */
-  class RegExpDollar extends RegExpSpecialChar {
+  class RegExpDollar extends RegExpAnchor {
     RegExpDollar() { this.getChar() = "$" }
 
     override string getPrimaryQLClass() { result = "RegExpDollar" }
@@ -946,7 +971,7 @@ module Impl implements RegexTreeViewSig {
    * ^
    * ```
    */
-  class RegExpCaret extends RegExpSpecialChar {
+  class RegExpCaret extends RegExpAnchor {
     RegExpCaret() { this.getChar() = "^" }
 
     override string getPrimaryQLClass() { result = "RegExpCaret" }

javascript/ql/lib/semmle/javascript/Regexp.qll

@@ -366,6 +366,9 @@ class RegExpAnchor extends RegExpTerm, @regexp_anchor {
   override predicate isNullable() { any() }
 
   override string getAPrimaryQlClass() { result = "RegExpAnchor" }
+
+  /** Gets the char for this term. */
+  abstract string getChar();
 }
 
 /**
@@ -379,6 +382,8 @@ class RegExpAnchor extends RegExpTerm, @regexp_anchor {
  */
 class RegExpCaret extends RegExpAnchor, @regexp_caret {
   override string getAPrimaryQlClass() { result = "RegExpCaret" }
+
+  override string getChar() { result = "^" }
 }
 
 /**
@@ -392,6 +397,8 @@ class RegExpCaret extends RegExpAnchor, @regexp_caret {
  */
 class RegExpDollar extends RegExpAnchor, @regexp_dollar {
   override string getAPrimaryQlClass() { result = "RegExpDollar" }
+
+  override string getChar() { result = "$" }
 }
 
 /**

javascript/ql/lib/semmle/javascript/security/regexp/HostnameRegexp.qll

@@ -8,7 +8,8 @@ private import semmle.javascript.security.regexp.RegExpTreeView::RegExpTreeView
 private import semmle.javascript.Regexp as RegExp
 private import codeql.regex.HostnameRegexp as Shared
 
-private module Impl implements Shared::HostnameRegexpSig<TreeImpl> {
+/** An implementation of the signature that allows the Hostname analysis to run. */
+module Impl implements Shared::HostnameRegexpSig<TreeImpl> {
   class DataFlowNode = JS::DataFlow::Node;
 
   class RegExpPatternSource = RegExp::RegExpPatternSource;

javascript/ql/src/Security/CWE-020/MissingRegExpAnchor.ql

@@ -12,199 +12,44 @@
  */
 
 private import javascript
-private import semmle.javascript.security.regexp.HostnameRegexp
-
-// TODO: Share the below code.
-/**
- * Holds if `term` is an anchor that is not the first or last node
- * in its tree.
- */
-predicate isInteriorAnchor(RegExpAnchor term) {
-  not isLeftArmTerm(term) and
-  not isRightArmTerm(term)
-}
-
-/**
- * Holds if `term` contains an anchor that is not the first or last node
- * in its tree, such as `(foo|bar$|baz)`.
- */
-predicate containsInteriorAnchor(RegExpTerm term) { isInteriorAnchor(term.getAChild*()) }
-
-/**
- * Holds if `term` starts with a word boundary or lookbehind assertion,
- * indicating that it's not intended to be anchored on that side.
- */
-predicate containsLeadingPseudoAnchor(RegExpSequence term) {
-  exists(RegExpTerm child | child = term.getChild(0) |
-    child instanceof RegExpWordBoundary or
-    child instanceof RegExpNonWordBoundary or
-    child instanceof RegExpLookbehind
-  )
-}
-
-/**
- * Holds if `term` ends with a word boundary or lookahead assertion,
- * indicating that it's not intended to be anchored on that side.
- */
-predicate containsTrailingPseudoAnchor(RegExpSequence term) {
-  exists(RegExpTerm child | child = term.getLastChild() |
-    child instanceof RegExpWordBoundary or
-    child instanceof RegExpNonWordBoundary or
-    child instanceof RegExpLookahead
-  )
-}
-
-/**
- * Holds if `term` is an empty sequence, usually arising from
- * literals with a trailing alternative such as `foo|`.
- */
-predicate isEmpty(RegExpSequence term) { term.getNumChild() = 0 }
-
-/**
- * Holds if `term` contains a letter constant.
- *
- * We use this as a heuristic to filter out uninteresting results.
- */
-predicate containsLetters(RegExpTerm term) {
-  term.getAChild*().(RegExpConstant).getValue().regexpMatch(".*[a-zA-Z].*")
-}
-
-/**
- * Holds if `term` consists only of an anchor and a parenthesized term,
- * such as the left side of `^(foo|bar)|baz`.
- *
- * The precedence of the anchor is likely to be intentional in this case,
- * as the group wouldn't be needed otherwise.
- */
-predicate isAnchoredGroup(RegExpSequence term) {
-  term.getNumChild() = 2 and
-  term.getAChild() instanceof RegExpAnchor and
-  term.getAChild() instanceof RegExpGroup
-}
-
-/**
- * Holds if `alt` has an explicitly anchored group, such as `^(foo|bar)|baz`
- * and doesn't have any unnecessary groups, such as in `^(foo)|(bar)`.
- */
-predicate hasExplicitAnchorPrecedence(RegExpAlt alt) {
-  isAnchoredGroup(alt.getAChild()) and
-  not alt.getAChild() instanceof RegExpGroup
-}
-
-/**
- * Holds if `src` is a pattern for a collection of alternatives where
- * only the first or last alternative is anchored, indicating a
- * precedence mistake explained by `msg`.
- *
- * The canonical example of such a mistake is: `^a|b|c`, which is
- * parsed as `(^a)|(b)|(c)`.
- */
-predicate hasMisleadingAnchorPrecedence(RegExpPatternSource src, string msg) {
-  exists(RegExpAlt root, RegExpSequence anchoredTerm, string direction |
-    root = src.getRegExpTerm() and
-    not containsInteriorAnchor(root) and
-    not isEmpty(root.getAChild()) and
-    not hasExplicitAnchorPrecedence(root) and
-    containsLetters(anchoredTerm) and
-    (
-      anchoredTerm = root.getChild(0) and
-      anchoredTerm.getChild(0) instanceof RegExpCaret and
-      not containsLeadingPseudoAnchor(root.getChild([1 .. root.getNumChild() - 1])) and
-      containsLetters(root.getChild([1 .. root.getNumChild() - 1])) and
-      direction = "beginning"
-      or
-      anchoredTerm = root.getLastChild() and
-      anchoredTerm.getLastChild() instanceof RegExpDollar and
-      not containsTrailingPseudoAnchor(root.getChild([0 .. root.getNumChild() - 2])) and
-      containsLetters(root.getChild([0 .. root.getNumChild() - 2])) and
-      direction = "end"
-    ) and
-    // is not used for replace
-    not exists(DataFlow::MethodCallNode replace |
-      replace.getMethodName() = "replace" and
-      src.getARegExpObject().flowsTo(replace.getArgument(0))
-    ) and
-    msg =
-      "Misleading operator precedence. The subexpression '" + anchoredTerm.getRawValue() +
-        "' is anchored at the " + direction +
-        ", but the other parts of this regular expression are not"
-  )
-}
-
-/**
- * Holds if `term` is a final term, that is, no term will match anything after this one.
- */
-predicate isFinalRegExpTerm(RegExpTerm term) {
-  term.isRootTerm()
-  or
-  exists(RegExpSequence seq |
-    isFinalRegExpTerm(seq) and
-    term = seq.getLastChild()
-  )
-  or
-  exists(RegExpTerm parent |
-    isFinalRegExpTerm(parent) and
-    term = parent.getAChild() and
-    not parent instanceof RegExpSequence and
-    not parent instanceof RegExpQuantifier
-  )
-}
-
-/**
- * Holds if `src` contains a hostname pattern that is missing a `$` anchor.
- */
-predicate isSemiAnchoredHostnameRegExp(RegExpPatternSource src, string msg) {
-  not hasMisleadingAnchorPrecedence(src, _) and // avoid double reporting
-  exists(RegExpTerm term, RegExpSequence tld, int i | term = src.getRegExpTerm() |
-    not isConstantInvalidInsideOrigin(term.getAChild*()) and
-    tld = term.getAChild*() and
-    hasTopLevelDomainEnding(tld, i) and
-    isFinalRegExpTerm(tld.getChild(i)) and // nothing is matched after the TLD
-    tld.getChild(0) instanceof RegExpCaret and
-    msg =
-      "This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end."
-  )
-}
-
-/**
- * Holds if `src` is an unanchored pattern for a URL, indicating a
- * mistake explained by `msg`.
- */
-predicate isUnanchoredHostnameRegExp(RegExpPatternSource src, string msg) {
-  exists(RegExpTerm term, RegExpSequence tld | term = src.getRegExpTerm() |
-    alwaysMatchesHostname(term) and
-    tld = term.getAChild*() and
-    hasTopLevelDomainEnding(tld) and
-    not isConstantInvalidInsideOrigin(term.getAChild*()) and
-    not term.getAChild*() instanceof RegExpAnchor and
-    // that is not used for capture or replace
-    not exists(DataFlow::MethodCallNode mcn, string name | name = mcn.getMethodName() |
+private import semmle.javascript.security.regexp.HostnameRegexp as HostnameRegexp
+private import codeql.regex.MissingRegExpAnchor as MissingRegExpAnchor
+private import semmle.javascript.security.regexp.RegExpTreeView::RegExpTreeView as TreeImpl
+
+private module Impl implements
+MissingRegExpAnchor::MissingRegExpAnchorSig<TreeImpl, HostnameRegexp::Impl> {
+  predicate isUsedAsReplace(RegExpPatternSource pattern) {
+    // is used for capture or replace
+    exists(DataFlow::MethodCallNode mcn, string name | name = mcn.getMethodName() |
       name = "exec" and
-      mcn = src.getARegExpObject().getAMethodCall() and
+      mcn = pattern.getARegExpObject().getAMethodCall() and
       exists(mcn.getAPropertyRead())
       or
       exists(DataFlow::Node arg |
         arg = mcn.getArgument(0) and
         (
-          src.getARegExpObject().flowsTo(arg) or
-          src.getAParse() = arg
+          pattern.getARegExpObject().flowsTo(arg) or
+          pattern.getAParse() = arg
         )
       |
         name = "replace"
         or
         name = "match" and exists(mcn.getAPropertyRead())
       )
-    ) and
-    msg =
-      "When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it."
-  )
+    )
+  }
+
+  string getEndAnchorText() { result = "$" }
 }
 
+import MissingRegExpAnchor::Make<TreeImpl, HostnameRegexp::Impl, Impl>
+
 from DataFlow::Node nd, string msg
 where
   isUnanchoredHostnameRegExp(nd, msg)
   or
   isSemiAnchoredHostnameRegExp(nd, msg)
   or
   hasMisleadingAnchorPrecedence(nd, msg)
+// isLineAnchoredHostnameRegExp is not used here, as it is not relevant to JS.
 select nd, msg

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

@@ -102,6 +102,9 @@ module Impl implements RegexTreeViewSig {
     /** Gets the number of child terms. */
     int getNumChild() { result = count(this.getAChild()) }
 
+    /** Gets the last child term of this element. */
+    RegExpTerm getLastChild() { result = this.getChild(this.getNumChild() - 1) }
+
     /** Gets the associated regex. */
     abstract Regex getRegex();
   }
@@ -561,6 +564,13 @@ module Impl implements RegexTreeViewSig {
     RegExpWordBoundary() { this.getChar() = "\\b" }
   }
 
+  /**
+   * A non-word boundary, that is, a regular expression term of the form `\B`.
+   */
+  class RegExpNonWordBoundary extends RegExpSpecialChar {
+    RegExpNonWordBoundary() { this.getChar() = "\\B" }
+  }
+
   /**
    * A character class escape in a regular expression.
    * That is, an escaped character that denotes multiple characters.
@@ -829,6 +839,19 @@ module Impl implements RegexTreeViewSig {
     override string getPrimaryQLClass() { result = "RegExpDot" }
   }
 
+  /**
+   * A term that matches a specific position between characters in the string.
+   *
+   * Example:
+   *
+   * ```
+   * \A
+   * ```
+   */
+  class RegExpAnchor extends RegExpSpecialChar {
+    RegExpAnchor() { this.getChar() = ["^", "$"] }
+  }
+
   /**
    * A dollar assertion `$` or `\Z` matching the end of a line.
    *
@@ -838,7 +861,7 @@ module Impl implements RegexTreeViewSig {
    * $
    * ```
    */
-  class RegExpDollar extends RegExpSpecialChar {
+  class RegExpDollar extends RegExpAnchor {
     RegExpDollar() { this.getChar() = ["$", "\\Z"] }
 
     override string getPrimaryQLClass() { result = "RegExpDollar" }
@@ -853,7 +876,7 @@ module Impl implements RegexTreeViewSig {
    * ^
    * ```
    */
-  class RegExpCaret extends RegExpSpecialChar {
+  class RegExpCaret extends RegExpAnchor {
     RegExpCaret() { this.getChar() = ["^", "\\A"] }
 
     override string getPrimaryQLClass() { result = "RegExpCaret" }