JS: review comments

Author: asger-semmle

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

@@ -242,12 +242,6 @@ module FlowLabel {
    * source, but not necessarily directly derived from it.
    */
   FlowLabel taint() { result = "taint" }
-
-  /**
-   * Gets one of the two standard flow labels, `data` or `taint`, describing values that originate
-   * from a flow source or are derived from a flow source.
-   */
-  FlowLabel dataOrTaint() { result = data() or result = taint() }
 }
 
 /**

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

@@ -23,15 +23,15 @@ module TaintedPath {
 
   module Label {
     /**
-     * String indicating if a path is normalized, that is, whether internal `../` components
+     * A string indicating if a path is normalized, that is, whether internal `../` components
      * have been removed.
      */
     class Normalization extends string {
       Normalization() { this = "normalized" or this = "raw" }
     }
 
     /**
-     * String indicating if a path is relative or absolute.
+     * A string indicating if a path is relative or absolute.
      */
     class Relativeness extends string {
       Relativeness() { this = "relative" or this = "absolute" }
@@ -108,7 +108,7 @@ module TaintedPath {
     PosixPath toPosixPath(DataFlow::FlowLabel label) {
       result = label
       or
-      label = DataFlow::FlowLabel::dataOrTaint()
+      label instanceof DataFlow::StandardFlowLabel
     }
   }
 
@@ -270,7 +270,7 @@ module TaintedPath {
    * Holds if `s` is a relative path.
    */
   bindingset[s]
-  private predicate isRelative(string s) { not s = "/" + any(string q) }
+  private predicate isRelative(string s) { not s.charAt(0) = "/" }
 
   /**
    * A call that normalizes a path.
@@ -375,7 +375,7 @@ module TaintedPath {
       input = getReceiver() and
       output = this and
       not exists(RegExpLiteral literal, RegExpSequence seq |
-        getArgument(0).asExpr() = literal and
+        getArgument(0).getALocalSource().asExpr() = literal and
         literal.isGlobal() and
         literal.getRoot() = seq and
         seq.getChild(0).(RegExpConstant).getValue() = "." and

javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js

@@ -223,11 +223,11 @@ app.get('/decode-after-normalization', (req, res) => {
 });
 
 app.get('/replace', (req, res) => {
-    let path = pathModule.normalize(req.query.path).replace(/%20/g, ' ');
-    if (!pathModule.isAbsolute(path)) {
-        res.sendFile(path); // NOT OK
+  let path = pathModule.normalize(req.query.path).replace(/%20/g, ' ');
+  if (!pathModule.isAbsolute(path)) {
+    res.sendFile(path); // NOT OK
 
-	    path = path.replace(/\.\./g, '');
-	    res.sendFile(path); // OK
-    }
+    path = path.replace(/\.\./g, '');
+    res.sendFile(path); // OK
+  }
 });