Merge pull request #8422 from erik-krogh/depMore

JS: Address some code that weren't affecting any query result

Author: erik-krogh

javascript/ql/lib/change-notes/2022-02-14-deprecated-predicates.md

@@ -0,0 +1,5 @@
+---
+category: deprecated
+---
+* Some predicates from `DefUse.qll`, `DataFlow.qll`, `TaintTracking.qll`, `DOM.qll`, `Definitions.qll` that weren't used by any query have been deprecated. 
+  The documentation for each predicate points to an alternative.

javascript/ql/lib/semmle/javascript/DefUse.qll

@@ -249,8 +249,9 @@ class VarUse extends ControlFlowNode, @varref {
 /**
  * Holds if the definition of `v` in `def` reaches `use` along some control flow path
  * without crossing another definition of `v`.
+ * DEPRECATED: Use the `SSA.qll` library instead.
  */
-predicate definitionReaches(Variable v, VarDef def, VarUse use) {
+deprecated predicate definitionReaches(Variable v, VarDef def, VarUse use) {
   v = use.getVariable() and
   exists(BasicBlock bb, int i, int next | next = nextDefAfter(bb, v, i, def) |
     exists(int j | j in [i + 1 .. next - 1] | bb.useAt(j, v, use))
@@ -265,24 +266,28 @@ predicate definitionReaches(Variable v, VarDef def, VarUse use) {
 /**
  * Holds if the definition of local variable `v` in `def` reaches `use` along some control flow path
  * without crossing another definition of `v`.
+ * DEPRECATED: Use the `SSA.qll` library instead.
  */
-predicate localDefinitionReaches(LocalVariable v, VarDef def, VarUse use) {
+deprecated predicate localDefinitionReaches(LocalVariable v, VarDef def, VarUse use) {
   exists(SsaExplicitDefinition ssa |
     ssa.defines(def, v) and
     ssa = getAPseudoDefinitionInput*(use.getSsaVariable().getDefinition())
   )
 }
 
-/** Holds if `nd` is a pseudo-definition and the result is one of its inputs. */
-private SsaDefinition getAPseudoDefinitionInput(SsaDefinition nd) {
+/**
+ * Holds if `nd` is a pseudo-definition and the result is one of its inputs.
+ * DEPRECATED: Use the `SSA.qll` library instead.
+ */
+deprecated private SsaDefinition getAPseudoDefinitionInput(SsaDefinition nd) {
   result = nd.(SsaPseudoDefinition).getAnInput()
 }
 
 /**
  * Holds if `d` is a definition of `v` at index `i` in `bb`, and the result is the next index
  * in `bb` after `i` at which the same variable is defined, or `bb.length()` if there is none.
  */
-private int nextDefAfter(BasicBlock bb, Variable v, int i, VarDef d) {
+deprecated private int nextDefAfter(BasicBlock bb, Variable v, int i, VarDef d) {
   bb.defAt(i, v, d) and
   result =
     min(int jj |
@@ -296,8 +301,9 @@ private int nextDefAfter(BasicBlock bb, Variable v, int i, VarDef d) {
  *
  * This is the case if there is a path from `earlier` to `later` that does not cross
  * another definition of `v`.
+ * DEPRECATED: Use the `SSA.qll` library instead.
  */
-predicate localDefinitionOverwrites(LocalVariable v, VarDef earlier, VarDef later) {
+deprecated predicate localDefinitionOverwrites(LocalVariable v, VarDef earlier, VarDef later) {
   exists(BasicBlock bb, int i, int next | next = nextDefAfter(bb, v, i, earlier) |
     bb.defAt(next, v, later)
     or

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

@@ -1718,5 +1718,5 @@ module DataFlow {
   import TypeTracking
   import internal.FunctionWrapperSteps
 
-  predicate localTaintStep = TaintTracking::localTaintStep/2;
+  deprecated predicate localTaintStep = TaintTracking::localTaintStep/2;
 }

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

@@ -429,9 +429,10 @@ module TaintTracking {
 
   /**
    * Holds if `pred -> succ` is a taint propagating data flow edge through a string operation.
+   * DEPRECATED: Use `stringConcatenationStep` and `stringManipulationStep` instead.
    */
   pragma[inline]
-  predicate stringStep(DataFlow::Node pred, DataFlow::Node succ) {
+  deprecated predicate stringStep(DataFlow::Node pred, DataFlow::Node succ) {
     stringConcatenationStep(pred, succ) or
     stringManipulationStep(pred, succ)
   }
@@ -1242,8 +1243,9 @@ module TaintTracking {
 
   /**
    * Holds if taint propagates from `pred` to `succ` in one local (intra-procedural) step.
+   * DEPRECATED: Use `TaintTracking::sharedTaintStep` and `DataFlow::Node::getALocalSource()` instead.
    */
-  predicate localTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+  deprecated predicate localTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
     DataFlow::localFlowStep(pred, succ) or
     sharedTaintStep(pred, succ)
   }

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

@@ -32,15 +32,22 @@ predicate isLocation(Expr e) {
 }
 
 /**
+ * DEPRECATED: Use DOM::documentRef() instead.
  * Gets a reference to the 'document' object.
  */
-DataFlow::SourceNode document() { result = DOM::documentRef() }
+deprecated DataFlow::SourceNode document() { result = DOM::documentRef() }
 
-/** Holds if `e` could refer to the `document` object. */
-predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) }
+/**
+ * DEPRECATED: Use DOM::documentRef() instead.
+ * Holds if `e` could refer to the `document` object.
+ */
+deprecated predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) }
 
-/** Holds if `e` could refer to the document URL. */
-predicate isDocumentUrl(Expr e) { e.flow() = DOM::locationSource() }
+/**
+ * DEPRECATED: Use DOM::locationSource() instead.
+ * Holds if `e` could refer to the document URL.
+ */
+deprecated predicate isDocumentUrl(Expr e) { e.flow() = DOM::locationSource() }
 
 /** DEPRECATED: Alias for isDocumentUrl */
 deprecated predicate isDocumentURL = isDocumentUrl/1;

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

@@ -26,6 +26,11 @@ class Configuration extends TaintTracking::Configuration {
     guard instanceof TaintedObject::SanitizerGuard
   }
 
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof Sanitizer
+  }
+
   override predicate isAdditionalFlowStep(
     DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
   ) {

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

@@ -19,6 +19,11 @@ class Configuration extends DataFlow::Configuration {
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 
+  override predicate isBarrier(DataFlow::Node node) {
+    super.isBarrier(node) or
+    node instanceof Sanitizer
+  }
+
   override predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) {
     exists(Base64::Encode encode | src = encode.getInput() and trg = encode.getOutput())
     or

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

@@ -31,4 +31,9 @@ class Configuration extends DataFlow::Configuration {
   override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
     sink.(Sink).getALabel() = label
   }
+
+  override predicate isBarrier(DataFlow::Node node) {
+    super.isBarrier(node) or
+    node instanceof Sanitizer
+  }
 }

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

@@ -34,6 +34,8 @@ module UnvalidatedDynamicMethodCall {
 
   /**
    * A sanitizer for unvalidated dynamic method calls.
+   * Override the `sanitizes` predicate to specify an edge that should be sanitized.
+   * The `this` value is not seen as a sanitizer.
    */
   abstract class Sanitizer extends DataFlow::Node {
     abstract predicate sanitizes(DataFlow::Node source, DataFlow::Node sink, DataFlow::FlowLabel lbl);

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

@@ -38,7 +38,11 @@ class Configuration extends TaintTracking::Configuration {
     sink.(Sink).getFlowLabel() = label
   }
 
-  override predicate isSanitizer(DataFlow::Node nd) { super.isSanitizer(nd) }
+  override predicate isSanitizerEdge(
+    DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel lbl
+  ) {
+    any(Sanitizer s).sanitizes(pred, succ, lbl)
+  }
 
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
     guard instanceof NumberGuard or

javascript/ql/src/Declarations/Definitions.qll

@@ -1,7 +1,10 @@
 import javascript
 
-/** An identifier appearing in a defining position. */
-class DefiningIdentifier extends Identifier {
+/**
+ * DEPRECATED: Use `SsaDefinition` from `SSA.qll` instead.
+ * An identifier appearing in a defining position.
+ */
+deprecated class DefiningIdentifier extends Identifier {
   DefiningIdentifier() {
     this instanceof VarDecl or
     exists(Assignment assgn | this = assgn.getLhs()) or

javascript/ql/src/experimental/semmle/javascript/security/dataflow/ResourceExhaustion.qll

@@ -23,6 +23,11 @@ module ResourceExhaustion {
 
     override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 
+    override predicate isSanitizer(DataFlow::Node node) {
+      super.isSanitizer(node) or
+      node instanceof Sanitizer
+    }
+
     override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node dst) {
       isNumericFlowStep(src, dst)
       or

javascript/ql/test/library-tests/Classes/AccessorMethods.qll

@@ -1,35 +1,30 @@
-| classes.js:1:1:2:1 | class Foo {\\n} | classes.js:4:1:4:3 | Foo |
-| classes.js:7:5:8:5 | class L ... {\\n    } | classes.js:10:5:10:12 | LocalFoo |
-| es2015.js:1:10:1:11 | fn | es2015.js:2:3:2:4 | fn |
-| es2015.js:5:16:5:16 | i | es2015.js:5:32:5:32 | i |
-| es2015.js:5:16:5:16 | i | es2015.js:5:34:5:34 | i |
-| es2015modules.js:1:10:1:12 | foo | es2015modules.js:4:3:4:5 | foo |
-| es2015modules.js:1:15:1:24 | bar as baz | es2015modules.js:6:3:6:5 | baz |
-| es2015modules.js:10:10:10:13 | quux | es2015modules.js:7:3:7:6 | quux |
-| es2015modules.js:15:17:15:17 | f | es2015modules.js:12:1:12:1 | f |
-| es2015modules.js:16:25:16:25 | g | es2015modules.js:13:1:13:1 | g |
-| fundecls.js:3:12:3:12 | f | fundecls.js:4:3:4:3 | f |
-| fundecls.js:9:10:9:10 | s | fundecls.js:7:1:7:1 | s |
-| fundecls.js:12:12:12:12 | f | fundecls.js:10:3:10:3 | f |
-| fundecls.js:18:12:18:12 | f | fundecls.js:17:3:17:3 | f |
-| fundecls.js:23:12:23:12 | f | fundecls.js:24:3:24:3 | f |
-| fundecls.js:34:12:34:12 | f | fundecls.js:35:3:35:3 | f |
-| fundecls.js:39:11:39:11 | x | fundecls.js:40:7:40:7 | x |
-| fundecls.js:41:14:41:14 | f | fundecls.js:45:3:45:3 | f |
-| fundecls.js:43:14:43:14 | f | fundecls.js:45:3:45:3 | f |
-| fundecls.js:48:11:48:11 | x | fundecls.js:50:7:50:7 | x |
-| tst.js:1:12:1:12 | o | tst.js:3:12:3:12 | o |
-| tst.js:1:12:1:12 | o | tst.js:5:16:5:16 | o |
-| tst.js:2:9:2:14 | y = 23 | tst.js:8:17:8:17 | y |
-| tst.js:2:17:2:21 | i = 0 | tst.js:4:5:4:5 | i |
-| tst.js:2:17:2:21 | i = 0 | tst.js:7:6:7:6 | i |
-| tst.js:4:3:4:5 | ++i | tst.js:4:5:4:5 | i |
-| tst.js:4:3:4:5 | ++i | tst.js:7:6:7:6 | i |
-| tst.js:5:11:5:11 | z | tst.js:6:7:6:7 | z |
-| tst.js:5:11:5:11 | z | tst.js:8:14:8:14 | z |
-| tst.js:7:4:7:6 | --i | tst.js:7:6:7:6 | i |
-| tst.js:12:2:12:7 | x = 42 | tst.js:14:9:14:9 | x |
-| tst.js:19:11:19:11 | x | tst.js:18:9:18:9 | x |
-| tst.js:23:6:23:23 | {a = b, c = d} = e | tst.js:24:2:24:2 | a |
-| tst.js:23:6:23:23 | {a = b, c = d} = e | tst.js:24:6:24:6 | c |
-| tst.js:26:11:26:11 | a | tst.js:27:2:27:2 | a |
+| classes.js:7:5:8:5 | def@7:5 | classes.js:10:5:10:12 | LocalFoo |
+| es2015.js:1:10:1:11 | def@1:10 | es2015.js:2:3:2:4 | fn |
+| es2015.js:5:16:5:16 | def@5:16 | es2015.js:5:32:5:32 | i |
+| es2015.js:5:16:5:16 | def@5:16 | es2015.js:5:34:5:34 | i |
+| es2015modules.js:1:10:1:12 | def@1:10 | es2015modules.js:4:3:4:5 | foo |
+| es2015modules.js:1:15:1:24 | def@1:15 | es2015modules.js:6:3:6:5 | baz |
+| es2015modules.js:10:10:10:13 | def@10:10 | es2015modules.js:7:3:7:6 | quux |
+| es2015modules.js:15:17:15:17 | def@15:17 | es2015modules.js:12:1:12:1 | f |
+| es2015modules.js:16:25:16:25 | def@16:25 | es2015modules.js:13:1:13:1 | g |
+| fundecls.js:3:12:3:12 | def@3:12 | fundecls.js:4:3:4:3 | f |
+| fundecls.js:12:12:12:12 | def@12:12 | fundecls.js:10:3:10:3 | f |
+| fundecls.js:18:12:18:12 | def@18:12 | fundecls.js:17:3:17:3 | f |
+| fundecls.js:23:12:23:12 | def@23:12 | fundecls.js:24:3:24:3 | f |
+| fundecls.js:27:2:27:2 | implicitInit@27:2 | fundecls.js:28:3:28:3 | f |
+| fundecls.js:34:12:34:12 | def@34:12 | fundecls.js:35:3:35:3 | f |
+| fundecls.js:39:11:39:11 | def@39:11 | fundecls.js:40:7:40:7 | x |
+| fundecls.js:45:3:45:3 | phi@45:3 | fundecls.js:45:3:45:3 | f |
+| fundecls.js:48:11:48:11 | def@48:11 | fundecls.js:50:7:50:7 | x |
+| tst.js:1:12:1:12 | def@1:12 | tst.js:3:12:3:12 | o |
+| tst.js:1:12:1:12 | def@1:12 | tst.js:5:16:5:16 | o |
+| tst.js:2:9:2:14 | def@2:9 | tst.js:8:17:8:17 | y |
+| tst.js:3:2:3:2 | phi@3:2 | tst.js:4:5:4:5 | i |
+| tst.js:5:2:5:2 | phi@5:2 | tst.js:7:6:7:6 | i |
+| tst.js:5:2:5:2 | phi@5:2 | tst.js:8:14:8:14 | z |
+| tst.js:5:11:5:11 | def@5:11 | tst.js:6:7:6:7 | z |
+| tst.js:12:2:12:7 | def@12:2 | tst.js:14:9:14:9 | x |
+| tst.js:19:11:19:11 | def@19:11 | tst.js:18:9:18:9 | x |
+| tst.js:23:6:23:23 | def@23:6 | tst.js:24:2:24:2 | a |
+| tst.js:23:6:23:23 | def@23:6 | tst.js:24:6:24:6 | c |
+| tst.js:26:11:26:11 | def@26:11 | tst.js:27:2:27:2 | a |

javascript/ql/test/library-tests/Classes/ClassDefinition_getName.qll

@@ -1,5 +1,5 @@
 import javascript
 
-from VarDef def, VarUse use
-where definitionReaches(_, def, use)
+from SsaVariable def, VarUse use
+where def.getAUse() = use
 select def, use