Merge branch 'main' into stdlib-FileSystemAccess-improvement

Author: RasmusWL

java/ql/lib/semmle/code/java/dataflow/internal/tainttracking3/TaintTrackingImpl.qll

@@ -109,33 +109,13 @@ abstract class Configuration extends DataFlow::Configuration {
   /** Holds if taint propagation into `node` is prohibited. */
   predicate isSanitizerIn(DataFlow::Node node) { none() }
 
-  /**
-   * Holds if taint propagation into `node` is prohibited when the flow state is
-   * `state`.
-   */
-  predicate isSanitizerIn(DataFlow::Node node, DataFlow::FlowState state) { none() }
-
-  final override predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowState state) {
-    this.isSanitizerIn(node, state)
-  }
-
   final override predicate isBarrierIn(DataFlow::Node node) { this.isSanitizerIn(node) }
 
   /** Holds if taint propagation out of `node` is prohibited. */
   predicate isSanitizerOut(DataFlow::Node node) { none() }
 
   final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
 
-  /**
-   * Holds if taint propagation out of `node` is prohibited when the flow state is
-   * `state`.
-   */
-  predicate isSanitizerOut(DataFlow::Node node, DataFlow::FlowState state) { none() }
-
-  final override predicate isBarrierOut(DataFlow::Node node, DataFlow::FlowState state) {
-    this.isSanitizerOut(node, state)
-  }
-
   /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
   predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
 

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -383,8 +383,8 @@ module API {
       exists(Node pred, Label::ApiLabel lbl, string predpath |
         Impl::edge(pred, lbl, this) and
         predpath = pred.getAPath(length - 1) and
-        exists(string space | if length = 1 then space = "" else space = " " |
-          result = "(" + lbl + space + predpath + ")" and
+        exists(string dot | if length = 1 then dot = "" else dot = "." |
+          result = predpath + dot + lbl and
           // avoid producing strings longer than 1MB
           result.length() < 1000 * 1000
         )
@@ -1330,22 +1330,22 @@ module API {
         /** Gets the EntryPoint associated with this label. */
         API::EntryPoint getEntryPoint() { result = e }
 
-        override string toString() { result = e }
+        override string toString() { result = "getASuccessor(Label::entryPoint(\"" + e + "\"))" }
       }
 
       /** A label that gets a promised value. */
       class LabelPromised extends ApiLabel, MkLabelPromised {
-        override string toString() { result = "promised" }
+        override string toString() { result = "getPromised()" }
       }
 
       /** A label that gets a rejected promise. */
       class LabelPromisedError extends ApiLabel, MkLabelPromisedError {
-        override string toString() { result = "promisedError" }
+        override string toString() { result = "getPromisedError()" }
       }
 
       /** A label that gets the return value of a function. */
       class LabelReturn extends ApiLabel, MkLabelReturn {
-        override string toString() { result = "return" }
+        override string toString() { result = "getReturn()" }
       }
 
       /** A label for a module. */
@@ -1357,12 +1357,13 @@ module API {
         /** Gets the module associated with this label. */
         string getMod() { result = mod }
 
-        override string toString() { result = "module " + mod }
+        // moduleImport is not neccesarilly the predicate to use, but it's close enough for most cases.
+        override string toString() { result = "moduleImport(\"" + mod + "\")" }
       }
 
       /** A label that gets an instance from a `new` call. */
       class LabelInstance extends ApiLabel, MkLabelInstance {
-        override string toString() { result = "instance" }
+        override string toString() { result = "getInstance()" }
       }
 
       /** A label for the member named `prop`. */
@@ -1374,14 +1375,14 @@ module API {
         /** Gets the property associated with this label. */
         string getProperty() { result = prop }
 
-        override string toString() { result = "member " + prop }
+        override string toString() { result = "getMember(\"" + prop + "\")" }
       }
 
       /** A label for a member with an unknown name. */
       class LabelUnknownMember extends ApiLabel, MkLabelUnknownMember {
         LabelUnknownMember() { this = MkLabelUnknownMember() }
 
-        override string toString() { result = "member *" }
+        override string toString() { result = "getUnknownMember()" }
       }
 
       /** A label for parameter `i`. */
@@ -1390,30 +1391,30 @@ module API {
 
         LabelParameter() { this = MkLabelParameter(i) }
 
-        override string toString() { result = "parameter " + i }
+        override string toString() { result = "getParameter(" + i + ")" }
 
         /** Gets the index of the parameter for this label. */
         int getIndex() { result = i }
       }
 
       /** A label for the receiver of call, that is, the value passed as `this`. */
       class LabelReceiver extends ApiLabel, MkLabelReceiver {
-        override string toString() { result = "receiver" }
+        override string toString() { result = "getReceiver()" }
       }
 
       /** A label for a class decorated by the current value. */
       class LabelDecoratedClass extends ApiLabel, MkLabelDecoratedClass {
-        override string toString() { result = "decorated-class" }
+        override string toString() { result = "getADecoratedClass()" }
       }
 
       /** A label for a method, field, or accessor decorated by the current value. */
       class LabelDecoratedMethod extends ApiLabel, MkLabelDecoratedMember {
-        override string toString() { result = "decorated-member" }
+        override string toString() { result = "decoratedMember()" }
       }
 
       /** A label for a parameter decorated by the current value. */
       class LabelDecoratedParameter extends ApiLabel, MkLabelDecoratedParameter {
-        override string toString() { result = "decorated-parameter" }
+        override string toString() { result = "decoratedParameter()" }
       }
     }
   }

javascript/ql/src/Security/CWE-915/examples/PrototypePollutingAssignment.js

@@ -1,6 +1,7 @@
 let express = require('express');
+let app = express()
 
-express.put('/todos/:id', (req, res) => {
+app.put('/todos/:id', (req, res) => {
     let id = req.params.id;
     let items = req.session.todos[id];
     if (!items) {

javascript/ql/src/Security/CWE-915/examples/PrototypePollutingAssignmentFixed.js

@@ -1,6 +1,7 @@
 let express = require('express');
+let app = express()
 
-express.put('/todos/:id', (req, res) => {
+app.put('/todos/:id', (req, res) => {
     let id = req.params.id;
     let items = req.session.todos.get(id);
     if (!items) {

javascript/ql/test/ApiGraphs/VerifyAssertions.qll

@@ -1,12 +1,11 @@
 /**
  * A test query that verifies assertions about the API graph embedded in source-code comments.
  *
- * An assertion is a comment of the form `def <path>` or `use <path>`, and asserts that
- * there is a def/use feature reachable from the root along the given path (described using
- * s-expression syntax), and its associated data-flow node must start on the same line as the
- * comment.
+ * An assertion is a comment of the form `def=<path>` or `use=<path>`, and asserts that
+ * there is a def/use feature reachable from the root along the given path, and its
+ * associated data-flow node must start on the same line as the comment.
  *
- * We also support negative assertions of the form `!def <path>` or `!use <path>`, which assert
+ * We also support negative assertions of the form `MISSING: def <path>` or `MISSING: use <path>`, which assert
  * that there _isn't_ a node with the given path on the same line.
  *
  * The query only produces output for failed assertions, meaning that it should have no output
@@ -39,44 +38,55 @@ private string getLoc(DataFlow::Node nd) {
  * An assertion matching a data-flow node against an API-graph feature.
  */
 class Assertion extends Comment {
-  string polarity;
   string expectedKind;
   string expectedLoc;
+  string path;
+  string polarity;
 
   Assertion() {
     exists(string txt, string rex |
       txt = this.getText().trim() and
-      rex = "(!?)(def|use) .*"
+      rex = ".*?((?:MISSING: )?)(def|use)=([\\w\\(\\)\"\\.\\-\\/\\@\\:]*).*"
     |
       polarity = txt.regexpCapture(rex, 1) and
       expectedKind = txt.regexpCapture(rex, 2) and
+      path = txt.regexpCapture(rex, 3) and
       expectedLoc = this.getFile().getAbsolutePath() + ":" + this.getLocation().getStartLine()
     )
   }
 
-  string getEdgeLabel(int i) { result = this.getText().regexpFind("(?<=\\()[^()]+", i, _).trim() }
+  string getEdgeLabel(int i) {
+    // matches a single edge. E.g. `getParameter(1)` or `getMember("foo")`.
+    // The lookbehind/lookahead ensure that the boundary is correct, that is
+    // either the edge is next to a ".", or it's the end of the string.
+    result = path.regexpFind("(?<=\\.|^)([\\w\\(\\)\"\\-\\/\\@\\:]+)(?=\\.|$)", i, _).trim()
+  }
 
   int getPathLength() { result = max(int i | exists(this.getEdgeLabel(i))) + 1 }
 
+  predicate isNegative() { polarity = "MISSING: " }
+
   API::Node lookup(int i) {
-    i = this.getPathLength() and
+    i = 0 and
     result = API::root()
     or
     result =
-      this.lookup(i + 1)
-          .getASuccessor(any(API::Label::ApiLabel label | label.toString() = this.getEdgeLabel(i)))
+      this.lookup(i - 1)
+          .getASuccessor(any(API::Label::ApiLabel label |
+              label.toString() = this.getEdgeLabel(i - 1)
+            ))
   }
 
-  predicate isNegative() { polarity = "!" }
+  API::Node lookup() { result = this.lookup(this.getPathLength()) }
 
-  predicate holds() { getLoc(getNode(this.lookup(0), expectedKind)) = expectedLoc }
+  predicate holds() { getLoc(getNode(this.lookup(), expectedKind)) = expectedLoc }
 
   string tryExplainFailure() {
     exists(int i, API::Node nd, string prefix, string suffix |
       nd = this.lookup(i) and
-      i > 0 and
-      not exists(this.lookup([0 .. i - 1])) and
-      prefix = nd + " has no outgoing edge labelled " + this.getEdgeLabel(i - 1) + ";" and
+      i < getPathLength() and
+      not exists(this.lookup([i + 1 .. getPathLength()])) and
+      prefix = nd + " has no outgoing edge labelled " + this.getEdgeLabel(i) + ";" and
       if exists(nd.getASuccessor())
       then
         suffix =
@@ -91,13 +101,13 @@ class Assertion extends Comment {
       result = prefix + " " + suffix
     )
     or
-    exists(API::Node nd, string kind | nd = this.lookup(0) |
+    exists(API::Node nd, string kind | nd = this.lookup() |
       exists(getNode(nd, kind)) and
       not exists(getNode(nd, expectedKind)) and
       result = "Expected " + expectedKind + " node, but found " + kind + " node."
     )
     or
-    exists(DataFlow::Node nd | nd = getNode(this.lookup(0), expectedKind) |
+    exists(DataFlow::Node nd | nd = getNode(this.lookup(), expectedKind) |
       not getLoc(nd) = expectedLoc and
       result = "Node not found on this line (but there is one on line " + min(getLoc(nd)) + ")."
     )

javascript/ql/test/ApiGraphs/argprops/index.js

@@ -1,6 +1,6 @@
 const assert = require("assert");
 
 let o = {
-    foo: 23 /* def (member foo (parameter 0 (member equal (member exports (module assert))))) */
+    foo: 23 // def=moduleImport("assert").getMember("exports").getMember("equal").getParameter(0).getMember("foo")
 };
 assert.equal(o, o);

javascript/ql/test/ApiGraphs/async-await/index.js

@@ -1,5 +1,5 @@
 const fs = require('fs-extra');
 
 module.exports.foo = async function foo() {
-    return await fs.copy('/tmp/myfile', '/tmp/mynewfile'); /* use (promised (return (member copy (member exports (module fs-extra))))) */ /* def (promised (return (member foo (member exports (module async-await))))) */
+    return await fs.copy('/tmp/myfile', '/tmp/mynewfile'); /* use=moduleImport("fs-extra").getMember("exports").getMember("copy").getReturn().getPromised()*/ /* def=moduleImport("async-await").getMember("exports").getMember("foo").getReturn().getPromised() */
 };

javascript/ql/test/ApiGraphs/async-await/tst.ts

@@ -5,5 +5,5 @@ async function readFileUtf8(path: string): Promise<string> {
 }
 
 async function test(path: string) {
-    await readFileUtf8(path); /* use (promised (return (member readFile (member exports (module fs/promises))))) */
+    await readFileUtf8(path); /* use=moduleImport("fs/promises").getMember("exports").getMember("readFile").getReturn() */
 }

javascript/ql/test/ApiGraphs/bound-args/index.js

@@ -1,24 +1,24 @@
 import bar from 'foo';
 
 let boundbar = bar.bind(
-    "receiver", // def (receiver (member default (member exports (module foo))))
-    "firstarg"  // def (parameter 0 (member default (member exports (module foo))))
+    "receiver", // def=moduleImport("foo").getMember("exports").getMember("default").getReceiver()
+    "firstarg"  // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(0)
 );
 boundbar(
-    "secondarg" // def (parameter 1 (member default (member exports (module foo))))
+    "secondarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1)
 )
 
 let boundbar2 = boundbar.bind(
-    "ignored", // !def (receiver (member default (member exports (module foo))))
-    "othersecondarg" // def (parameter 1 (member default (member exports (module foo))))
+    "ignored", // MISSING: def=moduleImport("foo").getMember("exports)".getMember("default").getReceiver()
+    "othersecondarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1)
 )
 boundbar2(
-    "thirdarg" // def (parameter 2 (member default (member exports (module foo))))
+    "thirdarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(2)
 )
 
 let bar2 = bar;
 for (var i = 0; i < 2; ++i)
     bar2 = bar2.bind(
         null,
-        i /* def (parameter 1 (member default (member exports (module foo)))) */ /* def (parameter 9 (member default (member exports (module foo)))) */
+        i /* def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1) */ /* def=moduleImport("foo").getMember("exports").getMember("default").getParameter(9) */
     );

javascript/ql/test/ApiGraphs/branching-flow/index.js

@@ -3,5 +3,5 @@ const fs = require('fs');
 exports.foo = function (cb) {
     if (!cb)
         cb = function () { };
-    cb(fs.readFileSync("/etc/passwd")); /* def (parameter 0 (parameter 0 (member foo (member exports (module branching-flow))))) */
+    cb(fs.readFileSync("/etc/passwd")); /* def=moduleImport("branching-flow").getMember("exports").getMember("foo").getParameter(0).getParameter(0) */
 };