add flowlabels to js/clear-text-logging

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

@@ -659,6 +659,20 @@ module TaintTracking {
     }
   }
 
+  /**
+   * A taint step through the NodeJS function `util.inspect(..)`.
+   */
+  class UtilInspectTaintStep extends AdditionalTaintStep, DataFlow::InvokeNode {
+    UtilInspectTaintStep() {
+      this =  DataFlow::moduleImport("util").getAMethodCall("inspect")
+    }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      succ = this and
+      this.(DataFlow::CallNode).getAnArgument() = pred
+    }
+  }
+
   /**
    * A conditional checking a tainted string against a regular expression, which is
    * considered to be a sanitizer for all configurations.

javascript/ql/src/semmle/javascript/security/dataflow/CleartextLogging.qll

@@ -13,7 +13,7 @@ module CleartextLogging {
   import CleartextLoggingCustomizations::CleartextLogging
 
   /**
-   * A dataflow tracking configuration for clear-text logging of sensitive information.
+   * A taint tracking configuration for clear-text logging of sensitive information.
    *
    * This configuration identifies flows from `Source`s, which are sources of
    * sensitive data, to `Sink`s, which is an abstract class representing all
@@ -21,36 +21,40 @@ module CleartextLogging {
    * added either by extending the relevant class, or by subclassing this configuration itself,
    * and amending the sources and sinks.
    */
-  class Configuration extends DataFlow::Configuration {
+  class Configuration extends TaintTracking::Configuration {
     Configuration() { this = "CleartextLogging" }
 
-    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+    override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
+      source.(Source).getLabel() = lbl
+    }
 
-    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+    override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel lbl) {
+      sink.(Sink).getLabel() = lbl
+    }
 
-    override predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof Barrier }
 
-    override predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) {
-      StringConcatenation::taintStep(src, trg)
-      or
-      exists(string name | name = "toString" or name = "valueOf" |
-        src.(DataFlow::SourceNode).getAMethodCall(name) = trg
-      )
-      or
+    override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel lbl) {
+      // Property reads do not propagate taint.
+      not lbl instanceof ProcessEnvLabel and 
+      succ.(DataFlow::PropRead).getBase() = pred
+    }
+       
+    override predicate isAdditionalFlowStep(
+      DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
+    ) {
+      trg.(DataFlow::PropRead).getBase() = src and
+      inlbl instanceof ProcessEnvLabel and 
+      outlbl.isData() 
+    }
+    
+    override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node trg) {
       // A taint propagating data flow edge through objects: a tainted write taints the entire object.
       exists(DataFlow::PropWrite write |
         write.getRhs() = src and
         trg.(DataFlow::SourceNode).flowsTo(write.getBase())
       )
       or
-      // Taint step through `util.inspect(..)` from Node.js
-      trg = DataFlow::moduleImport("util").getAMethodCall("inspect") and
-      trg.(DataFlow::CallNode).getAnArgument() = src
-      or
-      // Taint step through a `str.replace(..)` call.
-      trg.(DataFlow::MethodCallNode).getCalleeName() = "replace" and
-      trg.(DataFlow::MethodCallNode).getReceiver() = src
-      or
       // Taint through the arguments object.
       exists(DataFlow::CallNode call, Function f, VarAccess var |
         src = call.getAnArgument() and

javascript/ql/src/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll

@@ -15,12 +15,18 @@ module CleartextLogging {
   abstract class Source extends DataFlow::Node {
     /** Gets a string that describes the type of this data flow source. */
     abstract string describe();
+    
+    abstract DataFlow::FlowLabel getLabel();
   }
 
   /**
    * A data flow sink for clear-text logging of sensitive information.
    */
-  abstract class Sink extends DataFlow::Node { }
+  abstract class Sink extends DataFlow::Node {
+    DataFlow::FlowLabel getLabel() {
+      result.isDataOrTaint()
+    }
+  }
 
   /**
    * A barrier for clear-text logging of sensitive information.
@@ -107,6 +113,10 @@ module CleartextLogging {
     }
 
     override string describe() { result = "an access to " + name }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData()
+    }
   }
 
   /** An access to a variable or property that might contain a password. */
@@ -131,6 +141,10 @@ module CleartextLogging {
     }
 
     override string describe() { result = "an access to " + name }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData()
+    }
   }
 
   /** A call that might return a password. */
@@ -143,14 +157,28 @@ module CleartextLogging {
     }
 
     override string describe() { result = "a call to " + name }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData()
+    }
   }
 
 
-  private class ProcessEnvSource extends Source {
+  class ProcessEnvSource extends Source {
     ProcessEnvSource() {
       this = NodeJSLib::process().getAPropertyRead("env")
     }
 
     override string describe() { result = "process environment" }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData() or 
+      result instanceof ProcessEnvLabel
+    }
+  }
+  class ProcessEnvLabel extends DataFlow::FlowLabel{
+    ProcessEnvLabel() {
+      this = "processEnv"
+    }
   }
 }