Merge branch 'main' into shared-http-client-request

python/ql/lib/CHANGELOG.md

@@ -1,3 +1,16 @@
+## 0.5.4
+
+### Deprecated APIs
+
+* Many classes/predicates/modules with upper-case acronyms in their name have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.
+* The utility files previously in the `semmle.python.security.performance` package have been moved to the `semmle.python.security.regexp` package.  
+  The previous files still exist as deprecated aliases.
+
+### Minor Analysis Improvements
+
+* Most deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.
+
 ## 0.5.3
 
 ### Minor Analysis Improvements

python/ql/lib/change-notes/2022-05-25-redos-refac.md

@@ -1,5 +0,0 @@
----
-category: deprecated
----
-* The utility files previously in the `semmle.python.security.performance` package have been moved to the `semmle.python.security.regexp` package.  
-  The previous files still exist as deprecated aliases.

python/ql/lib/change-notes/2022-08-17-deleted-deprecations.md

@@ -1,6 +0,0 @@
----
-category: minorAnalysis
----
-* Most deprecated predicates/classes/modules that have been deprecated for over a year have been 
-deleted.
-

python/ql/lib/change-notes/released/0.5.4.md

@@ -0,0 +1,12 @@
+## 0.5.4
+
+### Deprecated APIs
+
+* Many classes/predicates/modules with upper-case acronyms in their name have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.
+* The utility files previously in the `semmle.python.security.performance` package have been moved to the `semmle.python.security.regexp` package.  
+  The previous files still exist as deprecated aliases.
+
+### Minor Analysis Improvements
+
+* Most deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

python/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.5.3
+lastReleaseVersion: 0.5.4

python/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/python-all
-version: 0.5.4-dev
+version: 0.5.5-dev
 groups: python
 dbscheme: semmlecode.python.dbscheme
 extractor: python

python/ql/lib/semmle/python/frameworks/Stdlib.qll

@@ -1854,16 +1854,22 @@ private module StdlibPrivate {
   deprecated API::Node cgiHTTPServer() { result = cgiHttpServer() }
 
   /** Provides models for the `CGIHTTPServer` module. */
-  module CGIHTTPServer {
+  module CgiHttpServer {
     /**
      * Provides models for the `CGIHTTPServer.CGIHTTPRequestHandler` class (Python 2 only).
      */
-    module CGIHTTPRequestHandler {
-      /** Gets a reference to the `CGIHTTPServer.CGIHTTPRequestHandler` class. */
+    module CgiHttpRequestHandler {
+      /** Gets a reference to the `CGIHTTPServer.CgiHttpRequestHandler` class. */
       API::Node classRef() { result = cgiHttpServer().getMember("CGIHTTPRequestHandler") }
     }
+
+    /** DEPRECATED: Alias for CgiHttpRequestHandler */
+    deprecated module CGIHTTPRequestHandler = CgiHttpRequestHandler;
   }
 
+  /** DEPRECATED: Alias for CgiHttpServer */
+  deprecated module CGIHTTPServer = CgiHttpServer;
+
   // ---------------------------------------------------------------------------
   // http (Python 3 only)
   // ---------------------------------------------------------------------------
@@ -1911,10 +1917,13 @@ private module StdlibPrivate {
        *
        * See https://docs.python.org/3.9/library/http.server.html#http.server.CGIHTTPRequestHandler.
        */
-      module CGIHTTPRequestHandler {
+      module CgiHttpRequestHandler {
         /** Gets a reference to the `http.server.CGIHTTPRequestHandler` class. */
         API::Node classRef() { result = server().getMember("CGIHTTPRequestHandler") }
       }
+
+      /** DEPRECATED: Alias for CgiHttpRequestHandler */
+      deprecated module CGIHTTPRequestHandler = CgiHttpRequestHandler;
     }
   }
 
@@ -1933,11 +1942,11 @@ private module StdlibPrivate {
           // Python 2
           BaseHttpServer::BaseHttpRequestHandler::classRef(),
           SimpleHttpServer::SimpleHttpRequestHandler::classRef(),
-          CGIHTTPServer::CGIHTTPRequestHandler::classRef(),
+          CgiHttpServer::CgiHttpRequestHandler::classRef(),
           // Python 3
           Http::Server::BaseHttpRequestHandler::classRef(),
           Http::Server::SimpleHttpRequestHandler::classRef(),
-          Http::Server::CGIHTTPRequestHandler::classRef()
+          Http::Server::CgiHttpRequestHandler::classRef()
         ].getASubclass*()
     }
 
@@ -2089,8 +2098,8 @@ private module StdlibPrivate {
      *
      * See https://docs.python.org/3.10/library/wsgiref.html#wsgiref.simple_server.WSGIRequestHandler.get_environ
      */
-    class WSGIEnvirontParameter extends RemoteFlowSource::Range, DataFlow::ParameterNode {
-      WSGIEnvirontParameter() {
+    class WsgiEnvirontParameter extends RemoteFlowSource::Range, DataFlow::ParameterNode {
+      WsgiEnvirontParameter() {
         exists(WsgirefSimpleServerApplication func |
           if func.isMethod()
           then this.getParameter() = func.getArg(1)
@@ -2103,6 +2112,9 @@ private module StdlibPrivate {
       }
     }
 
+    /** DEPRECATED: Alias for WsgiEnvirontParameter */
+    deprecated class WSGIEnvirontParameter = WsgiEnvirontParameter;
+
     /**
      * Gets a reference to the parameter of a `WsgirefSimpleServerApplication` that
      * takes the `start_response` function.

python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll

@@ -155,6 +155,22 @@ module ModelInput {
      */
     abstract predicate row(string row);
   }
+
+  /**
+   * A unit class for adding additional type variable model rows.
+   */
+  class TypeVariableModelCsv extends Unit {
+    /**
+     * Holds if `row` specifies a path through a type variable.
+     *
+     * A row of form,
+     * ```
+     * name;path
+     * ```
+     * means `path` can be substituted for a token `TypeVar[name]`.
+     */
+    abstract predicate row(string row);
+  }
 }
 
 private import ModelInput
@@ -182,6 +198,8 @@ private predicate summaryModel(string row) { any(SummaryModelCsv s).row(inverseP
 
 private predicate typeModel(string row) { any(TypeModelCsv s).row(inversePad(row)) }
 
+private predicate typeVariableModel(string row) { any(TypeVariableModelCsv s).row(inversePad(row)) }
+
 /** Holds if a source model exists for the given parameters. */
 predicate sourceModel(string package, string type, string path, string kind) {
   exists(string row |
@@ -219,7 +237,7 @@ private predicate summaryModel(
   )
 }
 
-/** Holds if an type model exists for the given parameters. */
+/** Holds if a type model exists for the given parameters. */
 private predicate typeModel(
   string package1, string type1, string package2, string type2, string path
 ) {
@@ -233,6 +251,15 @@ private predicate typeModel(
   )
 }
 
+/** Holds if a type variable model exists for the given parameters. */
+private predicate typeVariableModel(string name, string path) {
+  exists(string row |
+    typeVariableModel(row) and
+    row.splitAt(";", 0) = name and
+    row.splitAt(";", 1) = path
+  )
+}
+
 /**
  * Gets a package that should be seen as an alias for the given other `package`,
  * or the `package` itself.
@@ -253,7 +280,7 @@ private predicate isRelevantPackage(string package) {
     sourceModel(package, _, _, _) or
     sinkModel(package, _, _, _) or
     summaryModel(package, _, _, _, _, _) or
-    typeModel(package, _, _, _, _)
+    typeModel(_, _, package, _, _)
   ) and
   (
     Specific::isPackageUsed(package)
@@ -290,6 +317,8 @@ private class AccessPathRange extends AccessPath::Range {
       summaryModel(package, _, _, this, _, _) or
       summaryModel(package, _, _, _, this, _)
     )
+    or
+    typeVariableModel(_, this)
   }
 }
 
@@ -361,6 +390,72 @@ private API::Node getNodeFromPath(string package, string type, AccessPath path,
   // Similar to the other recursive case, but where the path may have stepped through one or more call-site filters
   result =
     getSuccessorFromInvoke(getInvocationFromPath(package, type, path, n - 1), path.getToken(n - 1))
+  or
+  // Apply a subpath
+  result =
+    getNodeFromSubPath(getNodeFromPath(package, type, path, n - 1), getSubPathAt(path, n - 1))
+  or
+  // Apply a type step
+  typeStep(getNodeFromPath(package, type, path, n), result)
+}
+
+/**
+ * Gets a subpath for the `TypeVar` token found at the `n`th token of `path`.
+ */
+pragma[nomagic]
+private AccessPath getSubPathAt(AccessPath path, int n) {
+  exists(string typeVarName |
+    path.getToken(n).getAnArgument("TypeVar") = typeVarName and
+    typeVariableModel(typeVarName, result)
+  )
+}
+
+/**
+ * Gets a node that is found by evaluating the first `n` tokens of `subPath` starting at `base`.
+ */
+pragma[nomagic]
+private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath, int n) {
+  exists(AccessPath path, int k |
+    base = [getNodeFromPath(_, _, path, k), getNodeFromSubPath(_, path, k)] and
+    subPath = getSubPathAt(path, k) and
+    result = base and
+    n = 0
+  )
+  or
+  exists(string package, string type, AccessPath basePath |
+    typeStepModel(package, type, basePath, subPath) and
+    base = getNodeFromPath(package, type, basePath) and
+    result = base and
+    n = 0
+  )
+  or
+  result = getSuccessorFromNode(getNodeFromSubPath(base, subPath, n - 1), subPath.getToken(n - 1))
+  or
+  result =
+    getSuccessorFromInvoke(getInvocationFromSubPath(base, subPath, n - 1), subPath.getToken(n - 1))
+  or
+  result =
+    getNodeFromSubPath(getNodeFromSubPath(base, subPath, n - 1), getSubPathAt(subPath, n - 1))
+  or
+  typeStep(getNodeFromSubPath(base, subPath, n), result)
+}
+
+/**
+ * Gets a call site that is found by evaluating the first `n` tokens of `subPath` starting at `base`.
+ */
+private Specific::InvokeNode getInvocationFromSubPath(API::Node base, AccessPath subPath, int n) {
+  result = Specific::getAnInvocationOf(getNodeFromSubPath(base, subPath, n))
+  or
+  result = getInvocationFromSubPath(base, subPath, n - 1) and
+  invocationMatchesCallSiteFilter(result, subPath.getToken(n - 1))
+}
+
+/**
+ * Gets a node that is found by evaluating `subPath` starting at `base`.
+ */
+pragma[nomagic]
+private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath) {
+  result = getNodeFromSubPath(base, subPath, subPath.getNumToken())
 }
 
 /** Gets the node identified by the given `(package, type, path)` tuple. */
@@ -368,6 +463,20 @@ API::Node getNodeFromPath(string package, string type, AccessPath path) {
   result = getNodeFromPath(package, type, path, path.getNumToken())
 }
 
+pragma[nomagic]
+private predicate typeStepModel(string package, string type, AccessPath basePath, AccessPath output) {
+  summaryModel(package, type, basePath, "", output, "type")
+}
+
+pragma[nomagic]
+private predicate typeStep(API::Node pred, API::Node succ) {
+  exists(string package, string type, AccessPath basePath, AccessPath output |
+    typeStepModel(package, type, basePath, output) and
+    pred = getNodeFromPath(package, type, basePath) and
+    succ = getNodeFromSubPath(pred, output)
+  )
+}
+
 /**
  * Gets an invocation identified by the given `(package, type, path)` tuple.
  *
@@ -390,7 +499,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa
  */
 bindingset[name]
 predicate isValidTokenNameInIdentifyingAccessPath(string name) {
-  name = ["Argument", "Parameter", "ReturnValue", "WithArity"]
+  name = ["Argument", "Parameter", "ReturnValue", "WithArity", "TypeVar"]
   or
   Specific::isExtraValidTokenNameInIdentifyingAccessPath(name)
 }
@@ -418,6 +527,9 @@ predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argume
   name = "WithArity" and
   argument.regexpMatch("\\d+(\\.\\.(\\d+)?)?")
   or
+  name = "TypeVar" and
+  exists(argument)
+  or
   Specific::isExtraValidTokenArgumentInIdentifyingAccessPath(name, argument)
 }
 
@@ -489,6 +601,8 @@ module ModelOutput {
       any(SummaryModelCsv csv).row(row) and kind = "summary" and expectedArity = 6
       or
       any(TypeModelCsv csv).row(row) and kind = "type" and expectedArity = 5
+      or
+      any(TypeVariableModelCsv csv).row(row) and kind = "type-variable" and expectedArity = 2
     |
       actualArity = count(row.indexOf(";")) + 1 and
       actualArity != expectedArity and
@@ -499,7 +613,7 @@ module ModelOutput {
     or
     // Check names and arguments of access path tokens
     exists(AccessPath path, AccessPathToken token |
-      isRelevantFullPath(_, _, path) and
+      (isRelevantFullPath(_, _, path) or typeVariableModel(_, path)) and
       token = path.getToken(_)
     |
       not isValidTokenNameInIdentifyingAccessPath(token.getName()) and

python/ql/lib/semmle/python/security/OverlyLargeRangeQuery.qll

@@ -173,7 +173,7 @@ module RangePrinter {
   }
 
   /** Gets the number of parts we should print for a given `range`. */
-  private int parts(OverlyWideRange range) { result = 1 + strictcount(cutoff(range, _)) }
+  private int parts(OverlyWideRange range) { result = 1 + count(cutoff(range, _)) }
 
   /** Holds if the given part of a range should span from `low` to `high`. */
   private predicate part(OverlyWideRange range, int part, string low, string high) {
@@ -238,8 +238,13 @@ module RangePrinter {
 
 /** Gets a char range that is overly large because of `reason`. */
 RegExpCharacterRange getABadRange(string reason, int priority) {
+  result instanceof OverlyWideRange and
   priority = 0 and
-  reason = "is equivalent to " + result.(OverlyWideRange).printEquivalent()
+  exists(string equiv | equiv = result.(OverlyWideRange).printEquivalent() |
+    if equiv.length() <= 50
+    then reason = "is equivalent to " + equiv
+    else reason = "is equivalent to " + equiv.substring(0, 50) + "..."
+  )
   or
   priority = 1 and
   exists(RegExpCharacterRange other |

python/ql/lib/semmle/python/security/regexp/ExponentialBackTracking.qll

@@ -115,6 +115,7 @@ private newtype TStatePair =
 private int rankState(State state) {
   state =
     rank[result](State s, Location l |
+      stateInsideBacktracking(s) and
       l = s.getRepr().getLocation()
     |
       s order by l.getStartLine(), l.getStartColumn(), s.toString()

python/ql/lib/semmle/python/security/regexp/NfaUtils.qll

@@ -93,8 +93,6 @@ class RegExpRoot extends RegExpTerm {
    * Holds if this root term is relevant to the ReDoS analysis.
    */
   predicate isRelevant() {
-    // there is at least one repetition
-    getRoot(any(InfiniteRepetitionQuantifier q)) = this and
     // is actually used as a RegExp
     this.isUsedAsRegExp() and
     // not excluded for library specific reasons
@@ -877,6 +875,101 @@ predicate isStartState(State state) {
  */
 signature predicate isCandidateSig(State state, string pump);
 
+/**
+ * Holds if `state` is a candidate for ReDoS.
+ */
+signature predicate isCandidateSig(State state);
+
+/**
+ * Predicates for constructing a prefix string that leads to a given state.
+ */
+module PrefixConstruction<isCandidateSig/1 isCandidate> {
+  /**
+   * Holds if `state` is the textually last start state for the regular expression.
+   */
+  private predicate lastStartState(State state) {
+    exists(RegExpRoot root |
+      state =
+        max(State s, Location l |
+          s = stateInRelevantRegexp() and
+          isStartState(s) and
+          getRoot(s.getRepr()) = root and
+          l = s.getRepr().getLocation()
+        |
+          s
+          order by
+            l.getStartLine(), l.getStartColumn(), s.getRepr().toString(), l.getEndColumn(),
+            l.getEndLine()
+        )
+    )
+  }
+
+  /**
+   * Holds if there exists any transition (Epsilon() or other) from `a` to `b`.
+   */
+  private predicate existsTransition(State a, State b) { delta(a, _, b) }
+
+  /**
+   * Gets the minimum number of transitions it takes to reach `state` from the `start` state.
+   */
+  int prefixLength(State start, State state) =
+    shortestDistances(lastStartState/1, existsTransition/2)(start, state, result)
+
+  /**
+   * Gets the minimum number of transitions it takes to reach `state` from the start state.
+   */
+  private int lengthFromStart(State state) { result = prefixLength(_, state) }
+
+  /**
+   * Gets a string for which the regular expression will reach `state`.
+   *
+   * Has at most one result for any given `state`.
+   * This predicate will not always have a result even if there is a ReDoS issue in
+   * the regular expression.
+   */
+  string prefix(State state) {
+    lastStartState(state) and
+    result = ""
+    or
+    // the search stops past the last redos candidate state.
+    lengthFromStart(state) <= max(lengthFromStart(any(State s | isCandidate(s)))) and
+    exists(State prev |
+      // select a unique predecessor (by an arbitrary measure)
+      prev =
+        min(State s, Location loc |
+          lengthFromStart(s) = lengthFromStart(state) - 1 and
+          loc = s.getRepr().getLocation() and
+          delta(s, _, state)
+        |
+          s
+          order by
+            loc.getStartLine(), loc.getStartColumn(), loc.getEndLine(), loc.getEndColumn(),
+            s.getRepr().toString()
+        )
+    |
+      // greedy search for the shortest prefix
+      result = prefix(prev) and delta(prev, Epsilon(), state)
+      or
+      not delta(prev, Epsilon(), state) and
+      result = prefix(prev) + getCanonicalEdgeChar(prev, state)
+    )
+  }
+
+  /**
+   * Gets a canonical char for which there exists a transition from `prev` to `next` in the NFA.
+   */
+  private string getCanonicalEdgeChar(State prev, State next) {
+    result =
+      min(string c | delta(prev, any(InputSymbol symbol | c = intersect(Any(), symbol)), next))
+  }
+
+  /** Gets a state within a regular expression that contains a candidate state. */
+  pragma[noinline]
+  State stateInRelevantRegexp() {
+    exists(State s | isCandidate(s) | getRoot(s.getRepr()) = getRoot(result.getRepr()))
+  }
+}
+
 /**
  * A module for pruning candidate ReDoS states.
  * The candidates are specified by the `isCandidate` signature predicate.
@@ -910,95 +1003,9 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
   /** Gets a state that can reach the `accept-any` state using only epsilon steps. */
   private State acceptsAnySuffix() { epsilonSucc*(result) = AcceptAnySuffix(_) }
 
-  /**
-   * Predicates for constructing a prefix string that leads to a given state.
-   */
-  private module PrefixConstruction {
-    /**
-     * Holds if `state` is the textually last start state for the regular expression.
-     */
-    private predicate lastStartState(State state) {
-      exists(RegExpRoot root |
-        state =
-          max(State s, Location l |
-            s = stateInPumpableRegexp() and
-            isStartState(s) and
-            getRoot(s.getRepr()) = root and
-            l = s.getRepr().getLocation()
-          |
-            s
-            order by
-              l.getStartLine(), l.getStartColumn(), s.getRepr().toString(), l.getEndColumn(),
-              l.getEndLine()
-          )
-      )
-    }
+  predicate isCandidateState(State s) { isReDoSCandidate(s, _) }
 
-    /**
-     * Holds if there exists any transition (Epsilon() or other) from `a` to `b`.
-     */
-    private predicate existsTransition(State a, State b) { delta(a, _, b) }
-
-    /**
-     * Gets the minimum number of transitions it takes to reach `state` from the `start` state.
-     */
-    int prefixLength(State start, State state) =
-      shortestDistances(lastStartState/1, existsTransition/2)(start, state, result)
-
-    /**
-     * Gets the minimum number of transitions it takes to reach `state` from the start state.
-     */
-    private int lengthFromStart(State state) { result = prefixLength(_, state) }
-
-    /**
-     * Gets a string for which the regular expression will reach `state`.
-     *
-     * Has at most one result for any given `state`.
-     * This predicate will not always have a result even if there is a ReDoS issue in
-     * the regular expression.
-     */
-    string prefix(State state) {
-      lastStartState(state) and
-      result = ""
-      or
-      // the search stops past the last redos candidate state.
-      lengthFromStart(state) <= max(lengthFromStart(any(State s | isReDoSCandidate(s, _)))) and
-      exists(State prev |
-        // select a unique predecessor (by an arbitrary measure)
-        prev =
-          min(State s, Location loc |
-            lengthFromStart(s) = lengthFromStart(state) - 1 and
-            loc = s.getRepr().getLocation() and
-            delta(s, _, state)
-          |
-            s
-            order by
-              loc.getStartLine(), loc.getStartColumn(), loc.getEndLine(), loc.getEndColumn(),
-              s.getRepr().toString()
-          )
-      |
-        // greedy search for the shortest prefix
-        result = prefix(prev) and delta(prev, Epsilon(), state)
-        or
-        not delta(prev, Epsilon(), state) and
-        result = prefix(prev) + getCanonicalEdgeChar(prev, state)
-      )
-    }
-
-    /**
-     * Gets a canonical char for which there exists a transition from `prev` to `next` in the NFA.
-     */
-    private string getCanonicalEdgeChar(State prev, State next) {
-      result =
-        min(string c | delta(prev, any(InputSymbol symbol | c = intersect(Any(), symbol)), next))
-    }
-
-    /** Gets a state within a regular expression that has a pumpable state. */
-    pragma[noinline]
-    State stateInPumpableRegexp() {
-      exists(State s | isReDoSCandidate(s, _) | getRoot(s.getRepr()) = getRoot(result.getRepr()))
-    }
-  }
+  import PrefixConstruction<isCandidateState/1> as Prefix
 
   /**
    * Predicates for testing the presence of a rejecting suffix.
@@ -1018,8 +1025,6 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
    * using epsilon transitions. But any attempt at repeating `w` will end in a state that accepts all suffixes.
    */
   private module SuffixConstruction {
-    import PrefixConstruction
-
     /**
      * Holds if all states reachable from `fork` by repeating `w`
      * are likely rejectable by appending some suffix.
@@ -1036,7 +1041,7 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
      */
     pragma[noinline]
     private predicate isLikelyRejectable(State s) {
-      s = stateInPumpableRegexp() and
+      s = Prefix::stateInRelevantRegexp() and
       (
         // exists a reject edge with some char.
         hasRejectEdge(s)
@@ -1052,7 +1057,7 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
      * Holds if `s` is not an accept state, and there is no epsilon transition to an accept state.
      */
     predicate isRejectState(State s) {
-      s = stateInPumpableRegexp() and not epsilonSucc*(s) = Accept(_)
+      s = Prefix::stateInRelevantRegexp() and not epsilonSucc*(s) = Accept(_)
     }
 
     /**
@@ -1060,7 +1065,7 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
      */
     pragma[noopt]
     predicate hasEdgeToLikelyRejectable(State s) {
-      s = stateInPumpableRegexp() and
+      s = Prefix::stateInRelevantRegexp() and
       // all edges (at least one) with some char leads to another state that is rejectable.
       // the `next` states might not share a common suffix, which can cause FPs.
       exists(string char | char = hasEdgeToLikelyRejectableHelper(s) |
@@ -1076,7 +1081,7 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
      */
     pragma[noinline]
     private string hasEdgeToLikelyRejectableHelper(State s) {
-      s = stateInPumpableRegexp() and
+      s = Prefix::stateInRelevantRegexp() and
       not hasRejectEdge(s) and
       not isRejectState(s) and
       deltaClosedChar(s, result, _)
@@ -1088,8 +1093,8 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
      * `prev` to `next` that the character symbol `char`.
      */
     predicate deltaClosedChar(State prev, string char, State next) {
-      prev = stateInPumpableRegexp() and
-      next = stateInPumpableRegexp() and
+      prev = Prefix::stateInRelevantRegexp() and
+      next = Prefix::stateInRelevantRegexp() and
       deltaClosed(prev, getAnInputSymbolMatchingRelevant(char), next)
     }
 
@@ -1099,18 +1104,28 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
       result = getAnInputSymbolMatching(char)
     }
 
+    pragma[noinline]
+    RegExpRoot relevantRoot() {
+      exists(RegExpTerm term, State s |
+        s.getRepr() = term and isCandidateState(s) and result = term.getRootTerm()
+      )
+    }
+
     /**
      * Gets a char used for finding possible suffixes inside `root`.
      */
     pragma[noinline]
     private string relevant(RegExpRoot root) {
-      exists(ascii(result)) and exists(root)
-      or
-      exists(InputSymbol s | belongsTo(s, root) | result = intersect(s, _))
-      or
-      // The characters from `hasSimpleRejectEdge`. Only `\n` is really needed (as `\n` is not in the `ascii` relation).
-      // The three chars must be kept in sync with `hasSimpleRejectEdge`.
-      result = ["|", "\n", "Z"] and exists(root)
+      root = relevantRoot() and
+      (
+        exists(ascii(result)) and exists(root)
+        or
+        exists(InputSymbol s | belongsTo(s, root) | result = intersect(s, _))
+        or
+        // The characters from `hasSimpleRejectEdge`. Only `\n` is really needed (as `\n` is not in the `ascii` relation).
+        // The three chars must be kept in sync with `hasSimpleRejectEdge`.
+        result = ["|", "\n", "Z"] and exists(root)
+      )
     }
 
     /**
@@ -1208,12 +1223,12 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
   predicate hasReDoSResult(RegExpTerm t, string pump, State s, string prefixMsg) {
     isReDoSAttackable(t, pump, s) and
     (
-      prefixMsg = "starting with '" + escape(PrefixConstruction::prefix(s)) + "' and " and
-      not PrefixConstruction::prefix(s) = ""
+      prefixMsg = "starting with '" + escape(Prefix::prefix(s)) + "' and " and
+      not Prefix::prefix(s) = ""
       or
-      PrefixConstruction::prefix(s) = "" and prefixMsg = ""
+      Prefix::prefix(s) = "" and prefixMsg = ""
       or
-      not exists(PrefixConstruction::prefix(s)) and prefixMsg = ""
+      not exists(Prefix::prefix(s)) and prefixMsg = ""
     )
   }
 

python/ql/lib/semmle/python/xml/XML.qll

@@ -8,7 +8,7 @@ private class TXmlLocatable =
   @xmldtd or @xmlelement or @xmlattribute or @xmlnamespace or @xmlcomment or @xmlcharacters;
 
 /** An XML element that has a location. */
-class XMLLocatable extends @xmllocatable, TXmlLocatable {
+class XmlLocatable extends @xmllocatable, TXmlLocatable {
   /** Gets the source location for this element. */
   Location getLocation() { xmllocations(this, result) }
 
@@ -32,13 +32,16 @@ class XMLLocatable extends @xmllocatable, TXmlLocatable {
   string toString() { none() } // overridden in subclasses
 }
 
+/** DEPRECATED: Alias for XmlLocatable */
+deprecated class XMLLocatable = XmlLocatable;
+
 /**
- * An `XMLParent` is either an `XMLElement` or an `XMLFile`,
+ * An `XmlParent` is either an `XmlElement` or an `XmlFile`,
  * both of which can contain other elements.
  */
-class XMLParent extends @xmlparent {
-  XMLParent() {
-    // explicitly restrict `this` to be either an `XMLElement` or an `XMLFile`;
+class XmlParent extends @xmlparent {
+  XmlParent() {
+    // explicitly restrict `this` to be either an `XmlElement` or an `XmlFile`;
     // the type `@xmlparent` currently also includes non-XML files
     this instanceof @xmlelement or xmlEncoding(this, _)
   }
@@ -50,28 +53,28 @@ class XMLParent extends @xmlparent {
   string getName() { none() } // overridden in subclasses
 
   /** Gets the file to which this XML parent belongs. */
-  XMLFile getFile() { result = this or xmlElements(this, _, _, _, result) }
+  XmlFile getFile() { result = this or xmlElements(this, _, _, _, result) }
 
   /** Gets the child element at a specified index of this XML parent. */
-  XMLElement getChild(int index) { xmlElements(result, _, this, index, _) }
+  XmlElement getChild(int index) { xmlElements(result, _, this, index, _) }
 
   /** Gets a child element of this XML parent. */
-  XMLElement getAChild() { xmlElements(result, _, this, _, _) }
+  XmlElement getAChild() { xmlElements(result, _, this, _, _) }
 
   /** Gets a child element of this XML parent with the given `name`. */
-  XMLElement getAChild(string name) { xmlElements(result, _, this, _, _) and result.hasName(name) }
+  XmlElement getAChild(string name) { xmlElements(result, _, this, _, _) and result.hasName(name) }
 
   /** Gets a comment that is a child of this XML parent. */
-  XMLComment getAComment() { xmlComments(result, _, this, _) }
+  XmlComment getAComment() { xmlComments(result, _, this, _) }
 
   /** Gets a character sequence that is a child of this XML parent. */
-  XMLCharacters getACharactersSet() { xmlChars(result, _, this, _, _, _) }
+  XmlCharacters getACharactersSet() { xmlChars(result, _, this, _, _, _) }
 
-  /** Gets the depth in the tree. (Overridden in XMLElement.) */
+  /** Gets the depth in the tree. (Overridden in XmlElement.) */
   int getDepth() { result = 0 }
 
   /** Gets the number of child XML elements of this XML parent. */
-  int getNumberOfChildren() { result = count(XMLElement e | xmlElements(e, _, this, _, _)) }
+  int getNumberOfChildren() { result = count(XmlElement e | xmlElements(e, _, this, _, _)) }
 
   /** Gets the number of places in the body of this XML parent where text occurs. */
   int getNumberOfCharacterSets() { result = count(int pos | xmlChars(_, _, this, pos, _, _)) }
@@ -92,9 +95,12 @@ class XMLParent extends @xmlparent {
   string toString() { result = this.getName() }
 }
 
+/** DEPRECATED: Alias for XmlParent */
+deprecated class XMLParent = XmlParent;
+
 /** An XML file. */
-class XMLFile extends XMLParent, File {
-  XMLFile() { xmlEncoding(this, _) }
+class XmlFile extends XmlParent, File {
+  XmlFile() { xmlEncoding(this, _) }
 
   /** Gets a printable representation of this XML file. */
   override string toString() { result = this.getName() }
@@ -120,15 +126,21 @@ class XMLFile extends XMLParent, File {
   string getEncoding() { xmlEncoding(this, result) }
 
   /** Gets the XML file itself. */
-  override XMLFile getFile() { result = this }
+  override XmlFile getFile() { result = this }
 
   /** Gets a top-most element in an XML file. */
-  XMLElement getARootElement() { result = this.getAChild() }
+  XmlElement getARootElement() { result = this.getAChild() }
 
   /** Gets a DTD associated with this XML file. */
-  XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) }
+  XmlDtd getADtd() { xmlDTDs(result, _, _, _, this) }
+
+  /** DEPRECATED: Alias for getADtd */
+  deprecated XmlDtd getADTD() { result = this.getADtd() }
 }
 
+/** DEPRECATED: Alias for XmlFile */
+deprecated class XMLFile = XmlFile;
+
 /**
  * An XML document type definition (DTD).
  *
@@ -140,7 +152,7 @@ class XMLFile extends XMLParent, File {
  * <!ELEMENT lastName (#PCDATA)>
  * ```
  */
-class XMLDTD extends XMLLocatable, @xmldtd {
+class XmlDtd extends XmlLocatable, @xmldtd {
   /** Gets the name of the root element of this DTD. */
   string getRoot() { xmlDTDs(this, result, _, _, _) }
 
@@ -154,7 +166,7 @@ class XMLDTD extends XMLLocatable, @xmldtd {
   predicate isPublic() { not xmlDTDs(this, _, "", _, _) }
 
   /** Gets the parent of this DTD. */
-  XMLParent getParent() { xmlDTDs(this, _, _, _, result) }
+  XmlParent getParent() { xmlDTDs(this, _, _, _, result) }
 
   override string toString() {
     this.isPublic() and
@@ -165,6 +177,9 @@ class XMLDTD extends XMLLocatable, @xmldtd {
   }
 }
 
+/** DEPRECATED: Alias for XmlDtd */
+deprecated class XMLDTD = XmlDtd;
+
 /**
  * An XML element in an XML file.
  *
@@ -176,7 +191,7 @@ class XMLDTD extends XMLLocatable, @xmldtd {
  * </manifest>
  * ```
  */
-class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
+class XmlElement extends @xmlelement, XmlParent, XmlLocatable {
   /** Holds if this XML element has the given `name`. */
   predicate hasName(string name) { name = this.getName() }
 
@@ -184,10 +199,10 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
   override string getName() { xmlElements(this, result, _, _, _) }
 
   /** Gets the XML file in which this XML element occurs. */
-  override XMLFile getFile() { xmlElements(this, _, _, _, result) }
+  override XmlFile getFile() { xmlElements(this, _, _, _, result) }
 
   /** Gets the parent of this XML element. */
-  XMLParent getParent() { xmlElements(this, _, result, _, _) }
+  XmlParent getParent() { xmlElements(this, _, result, _, _) }
 
   /** Gets the index of this XML element among its parent's children. */
   int getIndex() { xmlElements(this, _, _, result, _) }
@@ -196,7 +211,7 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
   predicate hasNamespace() { xmlHasNs(this, _, _) }
 
   /** Gets the namespace of this XML element, if any. */
-  XMLNamespace getNamespace() { xmlHasNs(this, result, _) }
+  XmlNamespace getNamespace() { xmlHasNs(this, result, _) }
 
   /** Gets the index of this XML element among its parent's children. */
   int getElementPositionIndex() { xmlElements(this, _, _, result, _) }
@@ -205,10 +220,10 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
   override int getDepth() { result = this.getParent().getDepth() + 1 }
 
   /** Gets an XML attribute of this XML element. */
-  XMLAttribute getAnAttribute() { result.getElement() = this }
+  XmlAttribute getAnAttribute() { result.getElement() = this }
 
   /** Gets the attribute with the specified `name`, if any. */
-  XMLAttribute getAttribute(string name) { result.getElement() = this and result.getName() = name }
+  XmlAttribute getAttribute(string name) { result.getElement() = this and result.getName() = name }
 
   /** Holds if this XML element has an attribute with the specified `name`. */
   predicate hasAttribute(string name) { exists(this.getAttribute(name)) }
@@ -220,6 +235,9 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
   override string toString() { result = this.getName() }
 }
 
+/** DEPRECATED: Alias for XmlElement */
+deprecated class XMLElement = XmlElement;
+
 /**
  * An attribute that occurs inside an XML element.
  *
@@ -230,18 +248,18 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
  * android:versionCode="1"
  * ```
  */
-class XMLAttribute extends @xmlattribute, XMLLocatable {
+class XmlAttribute extends @xmlattribute, XmlLocatable {
   /** Gets the name of this attribute. */
   string getName() { xmlAttrs(this, _, result, _, _, _) }
 
   /** Gets the XML element to which this attribute belongs. */
-  XMLElement getElement() { xmlAttrs(this, result, _, _, _, _) }
+  XmlElement getElement() { xmlAttrs(this, result, _, _, _, _) }
 
   /** Holds if this attribute has a namespace. */
   predicate hasNamespace() { xmlHasNs(this, _, _) }
 
   /** Gets the namespace of this attribute, if any. */
-  XMLNamespace getNamespace() { xmlHasNs(this, result, _) }
+  XmlNamespace getNamespace() { xmlHasNs(this, result, _) }
 
   /** Gets the value of this attribute. */
   string getValue() { xmlAttrs(this, _, _, result, _, _) }
@@ -250,6 +268,9 @@ class XMLAttribute extends @xmlattribute, XMLLocatable {
   override string toString() { result = this.getName() + "=" + this.getValue() }
 }
 
+/** DEPRECATED: Alias for XmlAttribute */
+deprecated class XMLAttribute = XmlAttribute;
+
 /**
  * A namespace used in an XML file.
  *
@@ -259,23 +280,29 @@ class XMLAttribute extends @xmlattribute, XMLLocatable {
  * xmlns:android="http://schemas.android.com/apk/res/android"
  * ```
  */
-class XMLNamespace extends XMLLocatable, @xmlnamespace {
+class XmlNamespace extends XmlLocatable, @xmlnamespace {
   /** Gets the prefix of this namespace. */
   string getPrefix() { xmlNs(this, result, _, _) }
 
   /** Gets the URI of this namespace. */
-  string getURI() { xmlNs(this, _, result, _) }
+  string getUri() { xmlNs(this, _, result, _) }
+
+  /** DEPRECATED: Alias for getUri */
+  deprecated string getURI() { result = this.getUri() }
 
   /** Holds if this namespace has no prefix. */
   predicate isDefault() { this.getPrefix() = "" }
 
   override string toString() {
-    this.isDefault() and result = this.getURI()
+    this.isDefault() and result = this.getUri()
     or
-    not this.isDefault() and result = this.getPrefix() + ":" + this.getURI()
+    not this.isDefault() and result = this.getPrefix() + ":" + this.getUri()
   }
 }
 
+/** DEPRECATED: Alias for XmlNamespace */
+deprecated class XMLNamespace = XmlNamespace;
+
 /**
  * A comment in an XML file.
  *
@@ -285,17 +312,20 @@ class XMLNamespace extends XMLLocatable, @xmlnamespace {
  * <!-- This is a comment. -->
  * ```
  */
-class XMLComment extends @xmlcomment, XMLLocatable {
+class XmlComment extends @xmlcomment, XmlLocatable {
   /** Gets the text content of this XML comment. */
   string getText() { xmlComments(this, result, _, _) }
 
   /** Gets the parent of this XML comment. */
-  XMLParent getParent() { xmlComments(this, _, result, _) }
+  XmlParent getParent() { xmlComments(this, _, result, _) }
 
   /** Gets a printable representation of this XML comment. */
   override string toString() { result = this.getText() }
 }
 
+/** DEPRECATED: Alias for XmlComment */
+deprecated class XMLComment = XmlComment;
+
 /**
  * A sequence of characters that occurs between opening and
  * closing tags of an XML element, excluding other elements.
@@ -306,12 +336,12 @@ class XMLComment extends @xmlcomment, XMLLocatable {
  * <content>This is a sequence of characters.</content>
  * ```
  */
-class XMLCharacters extends @xmlcharacters, XMLLocatable {
+class XmlCharacters extends @xmlcharacters, XmlLocatable {
   /** Gets the content of this character sequence. */
   string getCharacters() { xmlChars(this, result, _, _, _, _) }
 
   /** Gets the parent of this character sequence. */
-  XMLParent getParent() { xmlChars(this, _, result, _, _, _) }
+  XmlParent getParent() { xmlChars(this, _, result, _, _, _) }
 
   /** Holds if this character sequence is CDATA. */
   predicate isCDATA() { xmlChars(this, _, _, _, 1, _) }
@@ -319,3 +349,6 @@ class XMLCharacters extends @xmlcharacters, XMLLocatable {
   /** Gets a printable representation of this XML character sequence. */
   override string toString() { result = this.getCharacters() }
 }
+
+/** DEPRECATED: Alias for XmlCharacters */
+deprecated class XMLCharacters = XmlCharacters;