Merge pull request #7632 from erik-krogh/CWE-862

Approved by esbena, felicitymay

javascript/ql/lib/semmle/javascript/security/PasswordInConfigurationFileQuery.qll

@@ -0,0 +1,50 @@
+/** Classses and predicates for reasoning about passwords in configuration files. */
+
+import javascript
+import semmle.javascript.RestrictedLocations
+import semmle.javascript.security.SensitiveActions
+
+/**
+ * Holds if some JSON or YAML file contains a property with name `key`
+ * and value `val`, where `valElement` is the entity corresponding to the
+ * value.
+ *
+ * The following are excluded by this predicate:
+ * - Dependencies in `package.json` files.
+ * - Values that look like template delimiters.
+ * - Files that appear to be API-specifications, dictonary, test, or example.
+ */
+predicate config(string key, string val, Locatable valElement) {
+  (
+    exists(JSONObject obj | not exists(PackageJSON p | obj = p.getADependenciesObject(_)) |
+      obj.getPropValue(key) = valElement and
+      val = valElement.(JSONString).getValue()
+    )
+    or
+    exists(YAMLMapping m, YAMLString keyElement |
+      m.maps(keyElement, valElement) and
+      key = keyElement.getValue() and
+      (
+        val = valElement.(YAMLString).getValue()
+        or
+        valElement.toString() = "" and
+        val = ""
+      )
+    )
+  ) and
+  // exclude possible templates
+  not val.regexpMatch(Templating::getDelimiterMatchingRegexp()) and
+  not exclude(valElement.getFile())
+}
+
+/**
+ * Holds if file `f` should be excluded because it looks like it may be
+ * an API specification, a dictionary file, or a test or example.
+ */
+predicate exclude(File f) {
+  f.getRelativePath().regexpMatch("(?i).*(^|/)(lang(uage)?s?|locales?|tests?|examples?|i18n)/.*")
+  or
+  f.getStem().regexpMatch("(?i)translations?")
+  or
+  f.getExtension().toLowerCase() = "raml"
+}

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

@@ -14,47 +14,12 @@
  */
 
 import javascript
-import semmle.javascript.RestrictedLocations
-import semmle.javascript.security.SensitiveActions
-
-/**
- * Holds if some JSON or YAML file contains a property with name `key`
- * and value `val`, where `valElement` is the entity corresponding to the
- * value.
- *
- * Dependencies in `package.json` files are excluded by this predicate.
- */
-predicate config(string key, string val, Locatable valElement) {
-  exists(JSONObject obj | not exists(PackageJSON p | obj = p.getADependenciesObject(_)) |
-    obj.getPropValue(key) = valElement and
-    val = valElement.(JSONString).getValue()
-  )
-  or
-  exists(YAMLMapping m, YAMLString keyElement |
-    m.maps(keyElement, valElement) and
-    key = keyElement.getValue() and
-    val = valElement.(YAMLString).getValue()
-  )
-}
-
-/**
- * Holds if file `f` should be excluded because it looks like it may be
- * an API specification, a dictionary file, or a test or example.
- */
-predicate exclude(File f) {
-  f.getRelativePath().regexpMatch("(?i).*(^|/)(lang(uage)?s?|locales?|tests?|examples?|i18n)/.*")
-  or
-  f.getStem().regexpMatch("(?i)translations?")
-  or
-  f.getExtension().toLowerCase() = "raml"
-}
+import semmle.javascript.security.PasswordInConfigurationFileQuery
 
 from string key, string val, Locatable valElement, string pwd
 where
   config(key, val, valElement) and
   val != "" and
-  // exclude possible templates
-  not val.regexpMatch(Templating::getDelimiterMatchingRegexp()) and
   (
     key.toLowerCase() = "password" and
     pwd = val and
@@ -66,6 +31,5 @@ where
     // look for `password=...`, but exclude `password=;`, `password="$(...)"`,
     // `password=%s` and `password==`
     pwd = val.regexpCapture("(?is).*password\\s*=\\s*(?!;|\"?[$`]|%s|=)(\\S+).*", 1)
-  ) and
-  not exclude(valElement.getFile())
+  )
 select valElement.(FirstLineOf), "Hard-coded password '" + pwd + "' in configuration file."

javascript/ql/src/Security/CWE-862/EmptyPasswordInConfigurationFile.qhelp

@@ -0,0 +1,16 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The use of an empty string as a password in a configuration file is not secure.</p>
+
+</overview>
+<recommendation>
+<p>Choose a strong password and encrypt it if it has to be stored in a configuration file.</p>
+
+</recommendation>
+<references>
+
+</references>
+</qhelp>

javascript/ql/src/Security/CWE-862/EmptyPasswordInConfigurationFile.ql

@@ -0,0 +1,27 @@
+/**
+ * @name Empty password in configuration file
+ * @description Failing to set a password reduces the security of your code.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision medium
+ * @id js/empty-password-in-configuration-file
+ * @tags security
+ *       external/cwe/cwe-258
+ *       external/cwe/cwe-862
+ */
+
+import javascript
+import semmle.javascript.security.PasswordInConfigurationFileQuery
+
+from string key, string val, Locatable valElement
+where
+  config(key, val, valElement) and
+  (
+    val = "" and key.toLowerCase() = "password"
+    or
+    key.toLowerCase() != "readme" and
+    // look for `password=;`, `password=`, `password=  `, `password==`.
+    val.regexpMatch("(?is).*password\\s*(==?|:)\\s*(\\\"\\\"|''|``|;|:)?\\s*($|;|&|]|\\n).*")
+  )
+select valElement.(FirstLineOf), "Empty password in configuration file."

javascript/ql/src/change-notes/2022-01-18-empty-password-in-configuration-file.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* A new query `js/empty-password-in-configuration-file` has been added. The query detects empty passwords in configuration files. The query is not run by default.

javascript/ql/test/query-tests/Security/CWE-862/EmptyPasswordInConfigurationFile.expected

@@ -0,0 +1,6 @@
+| tst.json:6:17:6:18 | "" | Empty password in configuration file. |
+| tst.json:12:15:12:39 | "userna ... word= " | Empty password in configuration file. |
+| tst.json:15:15:15:38 | "passwo ... me=foo" | Empty password in configuration file. |
+| tst.yml:10:13:10:12 |  | Empty password in configuration file. |
+| tst.yml:14:12:14:34 | passwor ... me=foo; | Empty password in configuration file. |
+| tst.yml:16:12:16:38 | passwor ... ="foo"; | Empty password in configuration file. |

javascript/ql/test/query-tests/Security/CWE-862/EmptyPasswordInConfigurationFile.qlref

@@ -0,0 +1 @@
+Security/CWE-862/EmptyPasswordInConfigurationFile.ql

javascript/ql/test/query-tests/Security/CWE-862/tst.js

@@ -0,0 +1 @@
+console.log("foo");

javascript/ql/test/query-tests/Security/CWE-862/tst.json

@@ -0,0 +1,17 @@
+[
+  {
+    "password": "$pwd"
+  },
+  {
+    "password": ""
+  },
+  {
+    "password": "ff"
+  },
+  {
+    "config": "username=foo&password= "
+  },
+  {
+    "config": "password=&username=foo"
+  }
+]

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

@@ -0,0 +1,16 @@
+steps:
+- script: |
+    PASSWORD="$(PASSWORD)" npm install
+    OTHER_PASSWORD=`get password` yarn install
+username: <%= ENV['USERNAME'] %>
+password: <%= ENV['PASSWORD'] %>
+password: change_me
+others: 
+ - one:
+   password:
+ - two: 
+   password: <%= ENV['OTHER_PASSWORD'] %>
+ - three:
+   config: password=;username=foo;
+ - four:
+   config: password="";username="foo";