Merge pull request #1875 from esben-semmle/js/blacklist-more-hardcoded-passwords

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.23/analysis-javascript.md

@@ -23,6 +23,8 @@
 | Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false-positive results | This rule now recognizes additional ways delimiters can be stripped away. |
 | Client-side cross-site scripting (`js/xss`) | More results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized. |
 | Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving functions that manipulate DOM event handler attributes are now recognized. |
+| Hard-coded credentials (`js/hardcoded-credentials`) | Fewer false-positive results | This rule now flags fewer password examples. |
+| Password in configuration file (`js/password-in-configuration-file`) | Fewer false-positive results | This rule now flags fewer password examples. |
 | Prototype pollution (`js/prototype-pollution`) | More results | The query now highlights vulnerable uses of jQuery and Angular, and the results are shown on LGTM by default. |
 | Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. |
 

javascript/ql/src/Security/CWE-313/PasswordInConfigurationFile.ql

@@ -13,6 +13,7 @@
 
 import javascript
 import semmle.javascript.RestrictedLocations
+import semmle.javascript.security.SensitiveActions
 
 /**
  * Holds if some JSON or YAML file contains a property with name `key`
@@ -56,7 +57,8 @@ where
     key.toLowerCase() = "password" and
     pwd = val and
     // exclude interpolations of environment variables
-    not val.regexpMatch("\\$.*|%.*%")
+    not val.regexpMatch("\\$.*|%.*%") and
+    not PasswordHeuristics::isDummyPassword(val)
     or
     key.toLowerCase() != "readme" and
     // look for `password=...`, but exclude `password=;`, `password="$(...)"`,

javascript/ql/src/Security/CWE-798/HardcodedCredentials.ql

@@ -22,8 +22,14 @@ where
   // use source value in message if it's available
   if source.getNode().asExpr() instanceof ConstantString
   then
-    value = "The hard-coded value \"" + source.getNode().getStringValue() +
-        "\""
+    exists(string val | val = source.getNode().getStringValue() |
+      // exclude dummy passwords
+      not (
+        sink.getNode().(Sink).(DefaultCredentialsSink).getKind() = "password" and
+        PasswordHeuristics::isDummyPassword(val)
+      ) and
+      value = "The hard-coded value \"" + val + "\""
+    )
   else value = "This hard-coded value"
 select source.getNode(), source, sink, value + " is used as $@.", sink.getNode(),
   sink.getNode().(Sink).getKind()

javascript/ql/src/semmle/javascript/security/SensitiveActions.qll

@@ -245,3 +245,21 @@ class CleartextPasswordExpr extends SensitiveExpr {
 
   override SensitiveExpr::Classification getClassification() { none() }
 }
+
+/**
+ * Provides heuristics for classifying passwords.
+ */
+module PasswordHeuristics {
+  /**
+   * Holds if `password` looks like a deliberately weak password that the user should change.
+   */
+  bindingset[password]
+  predicate isDummyPassword(string password) {
+    password.length() < 4
+    or
+    exists(string normalized | normalized = password.toLowerCase() |
+      count(normalized.charAt(_)) = 1 or
+      normalized.regexpMatch(".*(pass|test|sample|example|secret|root|admin|user|change|auth).*")
+    )
+  }
+}

javascript/ql/test/library-tests/SensitiveActions/DummyPasswords.expected

@@ -0,0 +1,9 @@
+|  | true |
+| XXXXXXXX | true |
+| abcdefgh | false |
+| admin | true |
+| change_me | true |
+| example_password | true |
+| insert-auth-from-gui | true |
+| root | true |
+| sOKY6ccizpmvF*32so%Q | false |

javascript/ql/test/library-tests/SensitiveActions/DummyPasswords.ql

@@ -0,0 +1,20 @@
+import javascript
+import semmle.javascript.security.SensitiveActions
+
+string getASamplePassword() {
+  result = "abcdefgh" or
+  result = "sOKY6ccizpmvF*32so%Q" or
+  result = "XXXXXXXX" or
+  result = "example_password" or
+  result = "change_me" or
+  result = "" or
+  result = "insert-auth-from-gui" or
+  result = "admin" or
+  result = "root"
+}
+
+from string password, boolean isDummy
+where
+  password = getASamplePassword() and
+  if PasswordHeuristics::isDummyPassword(password) then isDummy = true else isDummy = false
+select password, isDummy

javascript/ql/test/query-tests/Security/CWE-313/PasswordInConfigurationFile.expected

@@ -1,3 +1,3 @@
-| mysql-config.json:4:16:4:23 | "secret" | Hard-coded password 'secret' in configuration file. |
+| mysql-config.json:4:16:4:25 | "abcdefgh" | Hard-coded password 'abcdefgh' in configuration file. |
 | tst4.json:2:10:2:38 | "script ... ecret'" | Hard-coded password ''secret'' in configuration file. |
 | tst7.yml:2:9:2:6 | \| | Hard-coded password 'abc' in configuration file. |

javascript/ql/test/query-tests/Security/CWE-313/mysql-config.json

@@ -1,6 +1,6 @@
 {
   "host"     : "localhost",
   "user"     : "me",
-  "password" : "secret",
+  "password" : "abcdefgh",
   "database" : "my_db"
-}
+}

javascript/ql/test/query-tests/Security/CWE-313/tst.yml

@@ -4,3 +4,4 @@ steps:
     OTHER_PASSWORD=`get password` yarn install
 username: <%= ENV['USERNAME'] %>
 password: <%= ENV['PASSWORD'] %>
+password: change_me