Merge branch 'main' into javascript_xss_improvements

javascript/ql/lib/semmle/javascript/Actions.qll

@@ -93,6 +93,9 @@ module Actions {
 
     /** Gets the value of the `if` field in this job, if any. */
     JobIf getIf() { result.getJob() = this }
+
+    /** Gets the value of the `runs-on` field in this job. */
+    JobRunson getRunsOn() { result.getJob() = this }
   }
 
   /**
@@ -108,6 +111,19 @@ module Actions {
     Job getJob() { result = job }
   }
 
+  /**
+   * A `runs-on` within a job.
+   * See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idruns-on.
+   */
+  class JobRunson extends YamlNode, YamlScalar {
+    Job job;
+
+    JobRunson() { job.lookup("runs-on") = this }
+
+    /** Gets the step this field belongs to. */
+    Job getJob() { result = job }
+  }
+
   /**
    * A step within an Actions job.
    * See https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idsteps.

javascript/ql/lib/semmle/javascript/JSX.qll

@@ -70,7 +70,7 @@ class JsxElement extends JsxNode {
   override string getAPrimaryQlClass() { result = "JsxElement" }
 
   /**
-   * Holds if this JSX element is a HTML element.
+   * Holds if this JSX element is an HTML element.
    * That is, the name starts with a lowercase letter.
    */
   predicate isHtmlElement() { getName().regexpMatch("[a-z].*") }

javascript/ql/lib/semmle/javascript/PrintAst.qll

@@ -161,7 +161,7 @@ private module PrintJavaScript {
   /**
    * A print node representing an `ASTNode`.
    *
-   * Provides a default implemention that works for some (but not all) ASTNode's.
+   * Provides a default implementation that works for some (but not all) ASTNode's.
    * More specific subclasses can override this class to get more specific behavior.
    *
    * The more specific subclasses are mostly used aggregate the children of the `ASTNode`.

javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll

@@ -711,13 +711,31 @@ module TaintTracking {
     }
   }
 
+  /**
+   * Gets a local source of any part of the input to the given stringification `call`.
+   */
+  pragma[nomagic]
+  private DataFlow::Node getAJsonLocalInput(JsonStringifyCall call) {
+    result = call.getInput()
+    or
+    exists(DataFlow::SourceNode source |
+      source = pragma[only_bind_out](getAJsonLocalInput(call)).getALocalSource()
+    |
+      result = source.getAPropertyWrite().getRhs()
+      or
+      result = source.(DataFlow::ObjectLiteralNode).getASpreadProperty()
+      or
+      result = source.(DataFlow::ArrayCreationNode).getASpreadArgument()
+    )
+  }
+
   /**
    * A taint propagating data flow edge arising from JSON unparsing.
    */
   private class JsonStringifyTaintStep extends SharedTaintStep {
     override predicate serializeStep(DataFlow::Node pred, DataFlow::Node succ) {
       exists(JsonStringifyCall call |
-        pred = call.getArgument(0) and
+        pred = getAJsonLocalInput(call) and
         succ = call
       )
     }

javascript/ql/lib/semmle/javascript/frameworks/ClientRequests.qll

@@ -671,7 +671,7 @@ module ClientRequest {
     }
 
     /**
-     * Gets the response type corresponding to `getReponse()` but not
+     * Gets the response type corresponding to `getResponse()` but not
      * for explicitly typed calls like `getResponseJson()`.
      */
     string getAssignedResponseType() {

javascript/ql/lib/semmle/javascript/frameworks/Immutable.qll

@@ -7,7 +7,7 @@ import javascript
 /**
  * Provides classes implementing data-flow for Immutable.
  *
- * The implemention rely on the flowsteps implemented in `Collections.qll`.
+ * The implementation rely on the flowsteps implemented in `Collections.qll`.
  */
 private module Immutable {
   /**

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

@@ -544,7 +544,7 @@ private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath) {
 }
 
 /** Gets the node identified by the given `(package, type, path)` tuple. */
-API::Node getNodeFromPath(string package, string type, AccessPath path) {
+private API::Node getNodeFromPath(string package, string type, AccessPath path) {
   result = getNodeFromPath(package, type, path, path.getNumToken())
 }
 
@@ -567,7 +567,9 @@ private predicate typeStep(API::Node pred, API::Node succ) {
  *
  * Unlike `getNodeFromPath`, the `path` may end with one or more call-site filters.
  */
-Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path, int n) {
+private Specific::InvokeNode getInvocationFromPath(
+  string package, string type, AccessPath path, int n
+) {
   result = Specific::getAnInvocationOf(getNodeFromPath(package, type, path, n))
   or
   result = getInvocationFromPath(package, type, path, n - 1) and
@@ -575,7 +577,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa
 }
 
 /** Gets an invocation identified by the given `(package, type, path)` tuple. */
-Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) {
+private Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPath path) {
   result = getInvocationFromPath(package, type, path, path.getNumToken())
 }
 
@@ -583,7 +585,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa
  * Holds if `name` is a valid name for an access path token in the identifying access path.
  */
 bindingset[name]
-predicate isValidTokenNameInIdentifyingAccessPath(string name) {
+private predicate isValidTokenNameInIdentifyingAccessPath(string name) {
   name = ["Argument", "Parameter", "ReturnValue", "WithArity", "TypeVar"]
   or
   Specific::isExtraValidTokenNameInIdentifyingAccessPath(name)
@@ -594,7 +596,7 @@ predicate isValidTokenNameInIdentifyingAccessPath(string name) {
  * in an identifying access path.
  */
 bindingset[name]
-predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) {
+private predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) {
   name = "ReturnValue"
   or
   Specific::isExtraValidNoArgumentTokenInIdentifyingAccessPath(name)
@@ -605,7 +607,7 @@ predicate isValidNoArgumentTokenInIdentifyingAccessPath(string name) {
  * in an identifying access path.
  */
 bindingset[name, argument]
-predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) {
+private predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argument) {
   name = ["Argument", "Parameter"] and
   argument.regexpMatch("(N-|-)?\\d+(\\.\\.((N-|-)?\\d+)?)?")
   or
@@ -622,51 +624,61 @@ predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argume
  * Module providing access to the imported models in terms of API graph nodes.
  */
 module ModelOutput {
-  /**
-   * Holds if a CSV source model contributed `source` with the given `kind`.
-   */
-  API::Node getASourceNode(string kind) {
-    exists(string package, string type, string path |
-      sourceModel(package, type, path, kind) and
-      result = getNodeFromPath(package, type, path)
-    )
+  cached
+  private module Cached {
+    /**
+     * Holds if a CSV source model contributed `source` with the given `kind`.
+     */
+    cached
+    API::Node getASourceNode(string kind) {
+      exists(string package, string type, string path |
+        sourceModel(package, type, path, kind) and
+        result = getNodeFromPath(package, type, path)
+      )
+    }
+
+    /**
+     * Holds if a CSV sink model contributed `sink` with the given `kind`.
+     */
+    cached
+    API::Node getASinkNode(string kind) {
+      exists(string package, string type, string path |
+        sinkModel(package, type, path, kind) and
+        result = getNodeFromPath(package, type, path)
+      )
+    }
+
+    /**
+     * Holds if a relevant CSV summary exists for these parameters.
+     */
+    cached
+    predicate relevantSummaryModel(
+      string package, string type, string path, string input, string output, string kind
+    ) {
+      isRelevantPackage(package) and
+      summaryModel(package, type, path, input, output, kind)
+    }
+
+    /**
+     * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row.
+     */
+    cached
+    predicate resolvedSummaryBase(
+      string package, string type, string path, Specific::InvokeNode baseNode
+    ) {
+      summaryModel(package, type, path, _, _, _) and
+      baseNode = getInvocationFromPath(package, type, path)
+    }
+
+    /**
+     * Holds if `node` is seen as an instance of `(package,type)` due to a type definition
+     * contributed by a CSV model.
+     */
+    cached
+    API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) }
   }
 
-  /**
-   * Holds if a CSV sink model contributed `sink` with the given `kind`.
-   */
-  API::Node getASinkNode(string kind) {
-    exists(string package, string type, string path |
-      sinkModel(package, type, path, kind) and
-      result = getNodeFromPath(package, type, path)
-    )
-  }
-
-  /**
-   * Holds if a relevant CSV summary exists for these parameters.
-   */
-  predicate relevantSummaryModel(
-    string package, string type, string path, string input, string output, string kind
-  ) {
-    isRelevantPackage(package) and
-    summaryModel(package, type, path, input, output, kind)
-  }
-
-  /**
-   * Holds if a `baseNode` is an invocation identified by the `package,type,path` part of a summary row.
-   */
-  predicate resolvedSummaryBase(
-    string package, string type, string path, Specific::InvokeNode baseNode
-  ) {
-    summaryModel(package, type, path, _, _, _) and
-    baseNode = getInvocationFromPath(package, type, path)
-  }
-
-  /**
-   * Holds if `node` is seen as an instance of `(package,type)` due to a type definition
-   * contributed by a CSV model.
-   */
-  API::Node getATypeNode(string package, string type) { result = getNodeFromType(package, type) }
+  import Cached
 
   /**
    * Gets an error message relating to an invalid CSV row in a model.

javascript/ql/lib/semmle/javascript/security/BadTagFilterQuery.qll

@@ -1,5 +1,5 @@
 /**
- * Provides precicates for reasoning about bad tag filter vulnerabilities.
+ * Provides predicates for reasoning about bad tag filter vulnerabilities.
  */
 
 import regexp.RegexpMatching
@@ -65,7 +65,7 @@ predicate isBadRegexpFilter(HtmlMatchingRegExp regexp, string msg) {
   regexp.matches("<!-- foo --!>") and
   exists(int a, int b | a != b |
     regexp.fillsCaptureGroup("<!-- foo -->", a) and
-    // <!-- foo --> might be ambigously parsed (matching both capture groups), and that is ok here.
+    // <!-- foo --> might be ambiguously parsed (matching both capture groups), and that is ok here.
     regexp.fillsCaptureGroup("<!-- foo --!>", b) and
     not regexp.fillsCaptureGroup("<!-- foo --!>", a) and
     msg =
@@ -87,7 +87,7 @@ predicate isBadRegexpFilter(HtmlMatchingRegExp regexp, string msg) {
     not regexp.fillsCaptureGroup("<script>", group) and
     msg =
       "This regular expression only parses --> (capture group " + group +
-        ") and not --!> as a HTML comment end tag."
+        ") and not --!> as an HTML comment end tag."
   )
   or
   regexp.matches("<!-- foo -->") and

javascript/ql/lib/semmle/javascript/security/IncompleteBlacklistSanitizer.qll

@@ -80,7 +80,7 @@ module HtmlSanitization {
   }
 
   /**
-   * Gets a HTML-relevant character that is replaced by `chain`.
+   * Gets an HTML-relevant character that is replaced by `chain`.
    */
   private string getALikelyReplacedCharacter(StringReplaceCallSequence chain) {
     result = "\"" and

javascript/ql/lib/semmle/javascript/security/IncompleteMultiCharacterSanitizationQuery.qll

@@ -35,7 +35,7 @@ private DangerousPrefixSubstring getADangerousMatchedChar(EmptyReplaceRegExpTerm
   or
   result = t.getAMatchedString()
   or
-  // A substring matched by some character class. This is only used to match the "word" part of a HTML tag (e.g. "iframe" in "<iframe").
+  // A substring matched by some character class. This is only used to match the "word" part of an HTML tag (e.g. "iframe" in "<iframe").
   exists(NfaUtils::CharacterClass cc |
     cc = NfaUtils::getCanonicalCharClass(t) and
     cc.matches(result) and
@@ -101,12 +101,12 @@ private class RepetitionMatcher extends EmptyReplaceRegExpTerm {
 predicate matchesDangerousPrefix(EmptyReplaceRegExpTerm t, string prefix, string kind) {
   prefix = getADangerousMatchedPrefix(t) and
   (
-    kind = "path injection" and
+    kind = "a path injection vulnerability" and
     prefix = ["/..", "../"] and
     // If the regex is matching explicit path components, it is unlikely that it's being used as a sanitizer.
     not t.getSuccessor*().getAMatchedString().regexpMatch("(?is).*[a-z0-9_-].*")
     or
-    kind = "HTML element injection" and
+    kind = "an HTML element injection vulnerability" and
     (
       // comments
       prefix = "<!--" and
@@ -119,7 +119,7 @@ predicate matchesDangerousPrefix(EmptyReplaceRegExpTerm t, string prefix, string
     )
   )
   or
-  kind = "HTML attribute injection" and
+  kind = "an HTML attribute injection vulnerability" and
   prefix =
     [
       // ordinary event handler prefix
@@ -197,6 +197,6 @@ query predicate problems(
 ) {
   exists(string kind |
     isResult(replace, dangerous, prefix, kind) and
-    msg = "This string may still contain $@, which may cause a " + kind + " vulnerability."
+    msg = "This string may still contain $@, which may cause " + kind + "."
   )
 }

javascript/ql/lib/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll

@@ -26,7 +26,7 @@ module ImproperCodeSanitization {
   abstract class Sanitizer extends DataFlow::Node { }
 
   /**
-   * A call to a HTML sanitizer seen as a source for improper code sanitization
+   * A call to an HTML sanitizer seen as a source for improper code sanitization
    */
   class HtmlSanitizerCallAsSource extends Source {
     HtmlSanitizerCallAsSource() { this instanceof HtmlSanitizerCall }

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll

@@ -32,7 +32,7 @@ module UnsafeJQueryPlugin {
   abstract class Sanitizer extends DataFlow::Node { }
 
   /**
-   * An argument that may act as a HTML fragment rather than a CSS selector, as a sink for remote unsafe jQuery plugins.
+   * An argument that may act as an HTML fragment rather than a CSS selector, as a sink for remote unsafe jQuery plugins.
    */
   class AmbiguousHtmlOrSelectorArgument extends DataFlow::Node,
     DomBasedXss::JQueryHtmlOrSelectorArgument {
@@ -173,7 +173,7 @@ module UnsafeJQueryPlugin {
   }
 
   /**
-   * An argument that may act as a HTML fragment rather than a CSS selector, as a sink for remote unsafe jQuery plugins.
+   * An argument that may act as an HTML fragment rather than a CSS selector, as a sink for remote unsafe jQuery plugins.
    */
   class AmbiguousHtmlOrSelectorArgumentAsSink extends Sink {
     AmbiguousHtmlOrSelectorArgumentAsSink() {
@@ -182,7 +182,7 @@ module UnsafeJQueryPlugin {
   }
 
   /**
-   * A hint that a value is expected to be treated as a HTML fragment later.
+   * A hint that a value is expected to be treated as an HTML fragment later.
    */
   class IntentionalHtmlFragmentHint extends Sanitizer {
     IntentionalHtmlFragmentHint() {
@@ -191,7 +191,7 @@ module UnsafeJQueryPlugin {
   }
 
   /**
-   * Holds if there exists a jQuery plugin that likely expects `sink` to be treated as a HTML fragment.
+   * Holds if there exists a jQuery plugin that likely expects `sink` to be treated as an HTML fragment.
    */
   predicate isLikelyIntentionalHtmlSink(DataFlow::Node sink) {
     exists(
@@ -206,7 +206,7 @@ module UnsafeJQueryPlugin {
   }
 
   /**
-   * Gets a property-write that writes a HTML-like constant string to `prop`.
+   * Gets a property-write that writes an HTML-like constant string to `prop`.
    */
   pragma[noinline]
   private DataFlow::PropWrite getALikelyHtmlWrite(string prop) {

javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll

@@ -65,7 +65,7 @@ module Shared {
   private import semmle.javascript.security.dataflow.IncompleteHtmlAttributeSanitizationCustomizations::IncompleteHtmlAttributeSanitization as IncompleteHtml
 
   /**
-   * A guard that checks if a string can contain quotes, which is a guard for strings that are inside a HTML attribute.
+   * A guard that checks if a string can contain quotes, which is a guard for strings that are inside an HTML attribute.
    */
   abstract class QuoteGuard extends TaintTracking::SanitizerGuardNode, StringOps::Includes {
     QuoteGuard() {

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

@@ -202,7 +202,7 @@ private predicate isFork(State q, InputSymbol s1, InputSymbol s2, State r1, Stat
     //
     // We additionally require that the there exists another InfiniteRepetitionQuantifier `mid` on the path from `q` to itself.
     // This is done to avoid flagging regular expressions such as `/(a?)*b/` - that only has polynomial runtime, and is detected by `js/polynomial-redos`.
-    // The below code is therefore a heuritic, that only flags regular expressions such as `/(a*)*b/`,
+    // The below code is therefore a heuristic, that only flags regular expressions such as `/(a*)*b/`,
     // and does not flag regular expressions such as `/(a?b?)c/`, but the latter pattern is not used frequently.
     r1 = r2 and
     q1 = q2 and

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

@@ -59,8 +59,8 @@ predicate matchesEpsilon(RegExpTerm t) {
 /**
  * A lookahead/lookbehind that matches the empty string.
  */
-class EmptyPositiveSubPatttern extends RegExpSubPattern {
-  EmptyPositiveSubPatttern() {
+class EmptyPositiveSubPattern extends RegExpSubPattern {
+  EmptyPositiveSubPattern() {
     (
       this instanceof RegExpPositiveLookahead
       or
@@ -70,6 +70,9 @@ class EmptyPositiveSubPatttern extends RegExpSubPattern {
   }
 }
 
+/** DEPRECATED: Use `EmptyPositiveSubPattern` instead. */
+deprecated class EmptyPositiveSubPatttern = EmptyPositiveSubPattern;
+
 /**
  * A branch in a disjunction that is the root node in a literal, or a literal
  * whose root node is not a disjunction.
@@ -133,7 +136,7 @@ private predicate isCanonicalTerm(RelevantRegExpTerm term, string str) {
 }
 
 /**
- * Gets a string reperesentation of the flags used with the regular expression.
+ * Gets a string representation of the flags used with the regular expression.
  * Only the flags that are relevant for the canonicalization are included.
  */
 string getCanonicalizationFlags(RegExpTerm root) {
@@ -334,7 +337,7 @@ private module CharacterClasses {
     )
   }
 
-  private string lowercaseLetter() { result = "abdcefghijklmnopqrstuvwxyz".charAt(_) }
+  private string lowercaseLetter() { result = "abcdefghijklmnopqrstuvwxyz".charAt(_) }
 
   private string upperCaseLetter() { result = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".charAt(_) }
 
@@ -697,9 +700,7 @@ predicate delta(State q1, EdgeLabel lbl, State q2) {
     lbl = Epsilon() and q2 = Accept(getRoot(dollar))
   )
   or
-  exists(EmptyPositiveSubPatttern empty | q1 = before(empty) |
-    lbl = Epsilon() and q2 = after(empty)
-  )
+  exists(EmptyPositiveSubPattern empty | q1 = before(empty) | lbl = Epsilon() and q2 = after(empty))
 }
 
 /**
@@ -1028,7 +1029,7 @@ module ReDoSPruning<isCandidateSig/2 isCandidate> {
    * as the suffix "X" will cause both the regular expressions to be rejected.
    *
    * The string `w` is repeated any number of times because it needs to be
-   * infinitely repeatedable for the attack to work.
+   * infinitely repeatable for the attack to work.
    * For the regular expression `/((ab)+)*abab/` the accepting state is not reachable from the fork
    * using epsilon transitions. But any attempt at repeating `w` will end in a state that accepts all suffixes.
    */

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

@@ -5,7 +5,7 @@
 import javascript
 
 /**
- * Holds if `term` is an ecape class representing e.g. `\d`.
+ * Holds if `term` is an escape class representing e.g. `\d`.
  * `clazz` is which character class it represents, e.g. "d" for `\d`.
  */
 predicate isEscapeClass(RegExpTerm term, string clazz) {
@@ -20,13 +20,13 @@ predicate isPossessive(RegExpQuantifier term) { none() }
 
 /**
  * Holds if the regex that `term` is part of is used in a way that ignores any leading prefix of the input it's matched against.
- * Not yet implemented for Javascript.
+ * Not yet implemented for JavaScript.
  */
 predicate matchesAnyPrefix(RegExpTerm term) { any() }
 
 /**
  * Holds if the regex that `term` is part of is used in a way that ignores any trailing suffix of the input it's matched against.
- * Not yet implemented for Javascript.
+ * Not yet implemented for JavaScript.
  */
 predicate matchesAnySuffix(RegExpTerm term) { any() }
 

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

@@ -1,5 +1,5 @@
 /**
- * Provides precicates for reasoning about which strings are matched by a regular expression,
+ * Provides predicates for reasoning about which strings are matched by a regular expression,
  * and for testing which capture groups are filled when a particular regexp matches a string.
  */
 

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

@@ -76,7 +76,7 @@ class StateTuple extends TStateTuple {
   StateTuple() { this = MkStateTuple(q1, q2, q3) }
 
   /**
-   * Gest a string repesentation of this tuple.
+   * Gest a string representation of this tuple.
    */
   string toString() { result = "(" + q1 + ", " + q2 + ", " + q3 + ")" }