Update qldoc and make the query more readable

java/ql/src/experimental/Security/CWE/CWE-555/CredentialsInPropertiesFile.qhelp

@@ -15,7 +15,7 @@
     <p>
       Credentials stored in properties files should be encrypted and recycled regularly. 
       In a Java EE deployment scenario, utilities provided by application servers like 
-      keystore and password vault can be used to encrypt and manage credentials.
+      keystores and password vaults can be used to encrypt and manage credentials.
     </p>
   </recommendation>
 
@@ -27,7 +27,7 @@
 
     <p>
       In the second example, the credentials of a LDAP and datasource properties are stored
-      in the encrypted format.
+      in an encrypted format.
     </p>
     <sample src="configuration.properties" />
   </example>

java/ql/src/experimental/Security/CWE/CWE-555/CredentialsInPropertiesFile.ql

@@ -9,10 +9,18 @@
  *       external/cwe/cwe-260
  */
 
+/*
+ * Note this query requires properties files to be indexed before it can produce results.
+ * If creating your own database with the CodeQL CLI, you should run
+ * `codeql database index-files --language=properties ...`
+ * If using lgtm.com, you should add `properties_files: true` to the index block of your
+ * lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction)
+ */
+
 import java
 import semmle.code.configfiles.ConfigFiles
 
-private string suspicious() {
+private string possibleSecretName() {
   result = "%password%" or
   result = "%passwd%" or
   result = "%account%" or
@@ -23,7 +31,7 @@ private string suspicious() {
   result = "%access%key%"
 }
 
-private string nonSuspicious() {
+private string possibleEncryptedSecretName() {
   result = "%hashed%" or
   result = "%encrypted%" or
   result = "%crypt%"
@@ -48,7 +56,8 @@ predicate isNotCleartextCredentials(string value) {
   or
   value.matches("ENC(%)") // Encrypted value
   or
-  value.toLowerCase().matches(suspicious()) // Could be message properties or fake passwords
+  // Could be a message property for UI display or fake passwords, e.g. login.password_expired=Your current password has expired.
+  value.toLowerCase().matches(possibleSecretName())
 }
 
 /**
@@ -57,8 +66,7 @@ predicate isNotCleartextCredentials(string value) {
  *    b) with a non-production file name
  */
 predicate isNonProdCredentials(CredentialsConfig cc) {
-  cc.getFile().getAbsolutePath().matches(["%dev%", "%test%", "%sample%"]) and
+  cc.getFile().getAbsolutePath().matches(["%dev%", "%test%", "%sample%"])
-  not cc.getFile().getAbsolutePath().matches("%codeql%") // CodeQL test cases
 }
 
 /** The properties file with configuration key/value pairs. */
@@ -69,8 +77,8 @@ class ConfigProperties extends ConfigPair {
 /** The credentials configuration property. */
 class CredentialsConfig extends ConfigProperties {
   CredentialsConfig() {
-    this.getNameElement().getName().trim().toLowerCase().matches(suspicious()) and
+    this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
-    not this.getNameElement().getName().trim().toLowerCase().matches(nonSuspicious())
+    not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName())
   }
 
   string getName() { result = this.getNameElement().getName().trim() }

java/ql/test/experimental/query-tests/security/CWE-555/CredentialsInPropertiesFile.ql

@@ -0,0 +1,84 @@
+/**
+ * @name Cleartext Credentials in Properties File
+ * @description Finds cleartext credentials in Java properties files.
+ * @kind problem
+ * @id java/credentials-in-properties
+ * @tags security
+ *       external/cwe/cwe-555
+ *       external/cwe/cwe-256
+ *       external/cwe/cwe-260
+ */
+
+/*
+ * Note this query requires properties files to be indexed before it can produce results.
+ * If creating your own database with the CodeQL CLI, you should run
+ * `codeql database index-files --language=properties ...`
+ * If using lgtm.com, you should add `properties_files: true` to the index block of your
+ * lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction)
+ */
+
+import java
+import semmle.code.configfiles.ConfigFiles
+
+private string possibleSecretName() {
+  result = "%password%" or
+  result = "%passwd%" or
+  result = "%account%" or
+  result = "%accnt%" or
+  result = "%credential%" or
+  result = "%token%" or
+  result = "%secret%" or
+  result = "%access%key%"
+}
+
+private string possibleEncryptedSecretName() {
+  result = "%hashed%" or
+  result = "%encrypted%" or
+  result = "%crypt%"
+}
+
+/** Holds if the value is not cleartext credentials. */
+bindingset[value]
+predicate isNotCleartextCredentials(string value) {
+  value = "" // Empty string
+  or
+  value.length() < 7 // Typical credentials are no less than 6 characters
+  or
+  value.matches("% %") // Sentences containing spaces
+  or
+  value.regexpMatch(".*[^a-zA-Z\\d]{3,}.*") // Contain repeated non-alphanumeric characters such as a fake password pass**** or ????
+  or
+  value.matches("@%") // Starts with the "@" sign
+  or
+  value.regexpMatch("\\$\\{.*\\}") // Variable placeholder ${credentials}
+  or
+  value.matches("%=") // A basic check of encrypted credentials ending with padding characters
+  or
+  value.matches("ENC(%)") // Encrypted value
+  or
+  // Could be a message property for UI display or fake passwords, e.g. login.password_expired=Your current password has expired.
+  value.toLowerCase().matches(possibleSecretName())
+}
+
+/** The properties file with configuration key/value pairs. */
+class ConfigProperties extends ConfigPair {
+  ConfigProperties() { this.getFile().getBaseName().toLowerCase().matches("%.properties") }
+}
+
+/** The credentials configuration property. */
+class CredentialsConfig extends ConfigProperties {
+  CredentialsConfig() {
+    this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
+    not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName())
+  }
+
+  string getName() { result = this.getNameElement().getName().trim() }
+
+  string getValue() { result = this.getValueElement().getValue().trim() }
+}
+
+from CredentialsConfig cc
+where not isNotCleartextCredentials(cc.getValue())
+select cc,
+  "Plaintext credentials " + cc.getName() + " have cleartext value " + cc.getValue() +
+    " in properties file."

java/ql/test/experimental/query-tests/security/CWE-555/CredentialsInPropertiesFile.qlref

@@ -1 +0,0 @@
-experimental/Security/CWE/CWE-555/CredentialsInPropertiesFile.ql