Merge pull request #837 from asger-semmle/hardcoded-empty-string

Approved by esben-semmle

Author: semmle-qlci

change-notes/1.20/analysis-javascript.md

@@ -27,6 +27,7 @@
 | **Query**                                  | **Expected impact**          | **Change**                                                                   |
 |--------------------------------------------|------------------------------|------------------------------------------------------------------------------|
 | Client-side cross-site scripting           | More true-positive results, fewer false-positive results.                 | This rule now recognizes WinJS functions that are vulnerable to HTML injection, and no longer flags certain safe uses of jQuery. |
+| Hard-coded credentials                     | Fewer false-positive results | This rule no longer flag the empty string as a hardcoded username. |
 | Insecure randomness | More results | This rule now flags insecure uses of `crypto.pseudoRandomBytes`. |
 | Uncontrolled data used in network request  | More results                 | This rule now recognizes host values that are vulnerable to injection. |
 | Unused parameter                           | Fewer false-positive results | This rule no longer flags parameters with leading underscore. |

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

@@ -41,9 +41,14 @@ module HardcodedCredentials {
    * A subclass of `Sink` that includes every `CredentialsExpr`
    * as a credentials sink.
    */
-  class DefaultCredentialsSink extends Sink {
-    DefaultCredentialsSink() { this.asExpr() instanceof CredentialsExpr }
+  class DefaultCredentialsSink extends Sink, DataFlow::ValueNode {
+    override CredentialsExpr astNode;
 
-    override string getKind() { result = this.asExpr().(CredentialsExpr).getCredentialsKind() }
+    DefaultCredentialsSink() {
+      // Don't flag an empty user name
+      not (astNode.getCredentialsKind() = "user name" and astNode.getStringValue() = "")
+    }
+
+    override string getKind() { result = astNode.getCredentialsKind() }
   }
 }

javascript/ql/test/query-tests/Security/CWE-798/HardcodedCredentials.js

@@ -134,3 +134,13 @@
 (function(){
     require("cookie-session")({ secret: "cookie-session secret" });
 })()
+
+(function(){
+    var request = require('request');
+    request.get(url, { // OK
+        'auth': {
+            'user': '',
+            'pass': process.env.PASSWORD
+        }
+    });
+})();