Merge pull request github#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