Update qldoc and enhance the query

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

@@ -1,5 +1,5 @@
-| configuration.properties:6:1:6:25 | ldap.password=mysecpass | Plaintext credentials ldap.password have cleartext value mysecpass in properties file. |
-| configuration.properties:18:1:18:35 | datasource1.password=Passw0rd@123 | Plaintext credentials datasource1.password have cleartext value Passw0rd@123 in properties file. |
-| configuration.properties:25:1:25:31 | mail.password=MysecPWxWa@1993 | Plaintext credentials mail.password have cleartext value MysecPWxWa@1993 in properties file. |
-| configuration.properties:33:1:33:50 | com.example.aws.s3.access_key=AKMAMQPBYMCD6YSAYCBA | Plaintext credentials com.example.aws.s3.access_key have cleartext value AKMAMQPBYMCD6YSAYCBA in properties file. |
-| configuration.properties:34:1:34:70 | com.example.aws.s3.secret_key=8lMPSfWzZq+wcWtck5+QPLOJDZzE783pS09/IO3k | Plaintext credentials com.example.aws.s3.secret_key have cleartext value 8lMPSfWzZq+wcWtck5+QPLOJDZzE783pS09/IO3k in properties file. |
+| configuration.properties:6:1:6:25 | ldap.password=mysecpass | Plaintext credentials ldap.password are loaded in getProperty(...) |
+| configuration.properties:18:1:18:35 | datasource1.password=Passw0rd@123 | Plaintext credentials datasource1.password are loaded in getProperty(...) |
+| configuration.properties:25:1:25:31 | mail.password=MysecPWxWa@1993 | Plaintext credentials mail.password are loaded in getProperty(...) |
+| configuration.properties:33:1:33:50 | com.example.aws.s3.access_key=AKMAMQPBYMCD6YSAYCBA | Plaintext credentials com.example.aws.s3.access_key are loaded in getProperty(...) |
+| configuration.properties:34:1:34:70 | com.example.aws.s3.secret_key=8lMPSfWzZq+wcWtck5+QPLOJDZzE783pS09/IO3k | Plaintext credentials com.example.aws.s3.secret_key are loaded in getProperty(...) |

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

@@ -14,28 +14,22 @@
  * 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)
+ * lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction#customizing-index)
  */
 
 import java
 import semmle.code.configfiles.ConfigFiles
+import semmle.code.java.dataflow.FlowSources
 
 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%"
+  result =
+    [
+      "%password%", "%passwd%", "%account%", "%accnt%", "%credential%", "%token%", "%secret%",
+      "%access%key%"
+    ]
 }
 
-private string possibleEncryptedSecretName() {
-  result = "%hashed%" or
-  result = "%encrypted%" or
-  result = "%crypt%"
-}
+private string possibleEncryptedSecretName() { result = ["%hashed%", "%encrypted%", "%crypt%"] }
 
 /** Holds if the value is not cleartext credentials. */
 bindingset[value]
@@ -60,25 +54,62 @@ predicate isNotCleartextCredentials(string value) {
   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 {
+class CredentialsConfig extends ConfigPair {
   CredentialsConfig() {
     this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
-    not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName())
+    not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName()) and
+    not isNotCleartextCredentials(this.getValueElement().getValue().trim())
   }
 
   string getName() { result = this.getNameElement().getName().trim() }
 
   string getValue() { result = this.getValueElement().getValue().trim() }
+
+  /** Returns a description of this vulnerability. */
+  string getConfigDesc() {
+    exists(
+      LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink, MethodAccess ma
+    |
+      this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
+      cc.hasFlow(source, sink) and
+      ma.getArgument(0) = sink.asExpr() and
+      result = "Plaintext credentials " + this.getName() + " are loaded in " + ma
+    )
+    or
+    not exists(LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink |
+      this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
+      cc.hasFlow(source, sink)
+    ) and
+    result =
+      "Plaintext credentials " + this.getName() + " have cleartext value " + this.getValue() +
+        " in properties file"
+  }
+}
+
+/**
+ * A dataflow configuration tracking flow of a method that loads a credentials property.
+ */
+class LoadCredentialsConfiguration extends DataFlow::Configuration {
+  LoadCredentialsConfiguration() { this = "LoadCredentialsConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(CredentialsConfig cc |
+      source.asExpr().(CompileTimeConstantExpr).getStringValue() = cc.getName()
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink.asExpr() =
+      any(MethodAccess ma |
+        ma.getMethod()
+            .getDeclaringType()
+            .getASupertype*()
+            .hasQualifiedName("java.util", "Properties") and
+        ma.getMethod().getName() = "getProperty"
+      ).getArgument(0)
+  }
 }
 
 from CredentialsConfig cc
-where not isNotCleartextCredentials(cc.getValue())
-select cc,
-  "Plaintext credentials " + cc.getName() + " have cleartext value " + cc.getValue() +
-    " in properties file."
+select cc, cc.getConfigDesc()

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

@@ -0,0 +1,57 @@
+import java.io.IOException;
+import java.util.Properties;
+
+public class PropertiesUtils {
+	/* Properties declaration. */ 
+	private static Properties properties;  
+
+    /** Static block to initializing the properties. */
+	static {
+		properties = new Properties();
+		try {
+			properties.load(PropertiesUtils.class.getClassLoader().getResourceAsStream("configuration.properties"));
+		} catch (IOException e) {
+			e.printStackTrace();
+		}
+	}
+
+	/** Returns the LDAP DN property value. */
+	public static String getLdapDN() {
+		return properties.getProperty("ldap.loginDN");
+	}
+    
+	/** Returns the LDAP password property value. */
+	public static String getLdapPassword() {
+		return properties.getProperty("ldap.password");
+	}
+
+	/** Returns the SQL Server username property value. */
+	public static String getMSDataSourceUserName() {
+		return properties.getProperty("datasource1.username");
+	}
+
+	/** Returns the SQL Server password property value. */
+	public static String getMSDataSourcePassword() {
+		return properties.getProperty("datasource1.password");
+	}
+    
+	/** Returns the mail account property value. */
+	public static String getMailUserName() {
+		return properties.getProperty("mail.username");
+	}
+
+ 	/** Returns the mail password property value. */
+	public static String getMailPassword() {
+		return properties.getProperty("mail.password");
+	}
+
+	/** Returns the AWS Access Key property value. */
+	public static String getAWSAccessKey() {
+		return properties.getProperty("com.example.aws.s3.access_key");
+	}
+
+	/** Returns the AWS Secret Key property value. */
+	public static String getAWSSecretKey() {
+		return properties.getProperty("com.example.aws.s3.secret_key");
+	}
+}

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

@@ -1,3 +1,4 @@
+# GOOD: UI display messages; not credentials
 prompt.username=Username
 prompt.password=Password