Skip to content

Commit 5ce3f9d

Browse files
luchua-bcluchua-bc
committed
Update qldoc and enhance the query
1 parent a53cbc1 commit 5ce3f9d

File tree

6 files changed

+179
-60
lines changed

6 files changed

+179
-60
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<overview>
44
<p>
55
Credentials management issues occur when credentials are stored in plaintext in
6-
an application’s properties file. Common credentials include but are not limited
6+
an application's properties file. Common credentials include but are not limited
77
to LDAP, mail, database, proxy account, and so on. Storing plaintext credentials
88
in a properties file allows anyone who can read the file access to the protected
99
resource. Good credentials management guidelines require that credentials never
@@ -21,12 +21,12 @@
2121

2222
<example>
2323
<p>
24-
In the first example, the credentials of a LDAP and datasource properties are stored
24+
In the first example, the credentials for the LDAP and datasource properties are stored
2525
in cleartext in the properties file.
2626
</p>
2727

2828
<p>
29-
In the second example, the credentials of a LDAP and datasource properties are stored
29+
In the second example, the credentials for the LDAP and datasource properties are stored
3030
in an encrypted format.
3131
</p>
3232
<sample src="configuration.properties" />

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

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,22 @@
1414
* If creating your own database with the CodeQL CLI, you should run
1515
* `codeql database index-files --language=properties ...`
1616
* If using lgtm.com, you should add `properties_files: true` to the index block of your
17-
* lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction)
17+
* lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction#customizing-index)
1818
*/
1919

2020
import java
2121
import semmle.code.configfiles.ConfigFiles
22+
import semmle.code.java.dataflow.FlowSources
2223

2324
private string possibleSecretName() {
24-
result = "%password%" or
25-
result = "%passwd%" or
26-
result = "%account%" or
27-
result = "%accnt%" or
28-
result = "%credential%" or
29-
result = "%token%" or
30-
result = "%secret%" or
31-
result = "%access%key%"
25+
result =
26+
[
27+
"%password%", "%passwd%", "%account%", "%accnt%", "%credential%", "%token%", "%secret%",
28+
"%access%key%"
29+
]
3230
}
3331

34-
private string possibleEncryptedSecretName() {
35-
result = "%hashed%" or
36-
result = "%encrypted%" or
37-
result = "%crypt%"
38-
}
32+
private string possibleEncryptedSecretName() { result = ["%hashed%", "%encrypted%", "%crypt%"] }
3933

4034
/** Holds if the value is not cleartext credentials. */
4135
bindingset[value]
@@ -69,27 +63,63 @@ predicate isNonProdCredentials(CredentialsConfig cc) {
6963
cc.getFile().getAbsolutePath().matches(["%dev%", "%test%", "%sample%"])
7064
}
7165

72-
/** The properties file with configuration key/value pairs. */
73-
class ConfigProperties extends ConfigPair {
74-
ConfigProperties() { this.getFile().getBaseName().toLowerCase().matches("%.properties") }
75-
}
76-
7766
/** The credentials configuration property. */
78-
class CredentialsConfig extends ConfigProperties {
67+
class CredentialsConfig extends ConfigPair {
7968
CredentialsConfig() {
8069
this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
81-
not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName())
70+
not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName()) and
71+
not isNotCleartextCredentials(this.getValueElement().getValue().trim())
8272
}
8373

8474
string getName() { result = this.getNameElement().getName().trim() }
8575

8676
string getValue() { result = this.getValueElement().getValue().trim() }
77+
78+
/** Returns a description of this vulnerability. */
79+
string getConfigDesc() {
80+
exists(
81+
LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink, MethodAccess ma
82+
|
83+
this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
84+
cc.hasFlow(source, sink) and
85+
ma.getArgument(0) = sink.asExpr() and
86+
result = "Plaintext credentials " + this.getName() + " are loaded in " + ma
87+
)
88+
or
89+
not exists(LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink |
90+
this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
91+
cc.hasFlow(source, sink)
92+
) and
93+
result =
94+
"Plaintext credentials " + this.getName() + " have cleartext value " + this.getValue() +
95+
" in properties file"
96+
}
97+
}
98+
99+
/**
100+
* A dataflow configuration tracking flow of a method that loads a credentials property.
101+
*/
102+
class LoadCredentialsConfiguration extends DataFlow::Configuration {
103+
LoadCredentialsConfiguration() { this = "LoadCredentialsConfiguration" }
104+
105+
override predicate isSource(DataFlow::Node source) {
106+
exists(CredentialsConfig cc |
107+
source.asExpr().(CompileTimeConstantExpr).getStringValue() = cc.getName()
108+
)
109+
}
110+
111+
override predicate isSink(DataFlow::Node sink) {
112+
sink.asExpr() =
113+
any(MethodAccess ma |
114+
ma.getMethod()
115+
.getDeclaringType()
116+
.getASupertype*()
117+
.hasQualifiedName("java.util", "Properties") and
118+
ma.getMethod().getName() = "getProperty"
119+
).getArgument(0)
120+
}
87121
}
88122

89123
from CredentialsConfig cc
90-
where
91-
not isNotCleartextCredentials(cc.getValue()) and
92-
not isNonProdCredentials(cc)
93-
select cc,
94-
"Plaintext credentials " + cc.getName() + " have cleartext value " + cc.getValue() +
95-
" in properties file."
124+
where not isNonProdCredentials(cc)
125+
select cc, cc.getConfigDesc()

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
| configuration.properties:6:1:6:25 | ldap.password=mysecpass | Plaintext credentials ldap.password have cleartext value mysecpass in properties file. |
2-
| configuration.properties:18:1:18:35 | datasource1.password=Passw0rd@123 | Plaintext credentials datasource1.password have cleartext value Passw0rd@123 in properties file. |
3-
| configuration.properties:25:1:25:31 | mail.password=MysecPWxWa@1993 | Plaintext credentials mail.password have cleartext value MysecPWxWa@1993 in properties file. |
4-
| 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. |
5-
| 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. |
1+
| configuration.properties:6:1:6:25 | ldap.password=mysecpass | Plaintext credentials ldap.password are loaded in getProperty(...) |
2+
| configuration.properties:18:1:18:35 | datasource1.password=Passw0rd@123 | Plaintext credentials datasource1.password are loaded in getProperty(...) |
3+
| configuration.properties:25:1:25:31 | mail.password=MysecPWxWa@1993 | Plaintext credentials mail.password are loaded in getProperty(...) |
4+
| 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(...) |
5+
| 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

Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,22 @@
1414
* If creating your own database with the CodeQL CLI, you should run
1515
* `codeql database index-files --language=properties ...`
1616
* If using lgtm.com, you should add `properties_files: true` to the index block of your
17-
* lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction)
17+
* lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction#customizing-index)
1818
*/
1919

2020
import java
2121
import semmle.code.configfiles.ConfigFiles
22+
import semmle.code.java.dataflow.FlowSources
2223

2324
private string possibleSecretName() {
24-
result = "%password%" or
25-
result = "%passwd%" or
26-
result = "%account%" or
27-
result = "%accnt%" or
28-
result = "%credential%" or
29-
result = "%token%" or
30-
result = "%secret%" or
31-
result = "%access%key%"
25+
result =
26+
[
27+
"%password%", "%passwd%", "%account%", "%accnt%", "%credential%", "%token%", "%secret%",
28+
"%access%key%"
29+
]
3230
}
3331

34-
private string possibleEncryptedSecretName() {
35-
result = "%hashed%" or
36-
result = "%encrypted%" or
37-
result = "%crypt%"
38-
}
32+
private string possibleEncryptedSecretName() { result = ["%hashed%", "%encrypted%", "%crypt%"] }
3933

4034
/** Holds if the value is not cleartext credentials. */
4135
bindingset[value]
@@ -60,25 +54,62 @@ predicate isNotCleartextCredentials(string value) {
6054
value.toLowerCase().matches(possibleSecretName())
6155
}
6256

63-
/** The properties file with configuration key/value pairs. */
64-
class ConfigProperties extends ConfigPair {
65-
ConfigProperties() { this.getFile().getBaseName().toLowerCase().matches("%.properties") }
66-
}
67-
6857
/** The credentials configuration property. */
69-
class CredentialsConfig extends ConfigProperties {
58+
class CredentialsConfig extends ConfigPair {
7059
CredentialsConfig() {
7160
this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
72-
not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName())
61+
not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName()) and
62+
not isNotCleartextCredentials(this.getValueElement().getValue().trim())
7363
}
7464

7565
string getName() { result = this.getNameElement().getName().trim() }
7666

7767
string getValue() { result = this.getValueElement().getValue().trim() }
68+
69+
/** Returns a description of this vulnerability. */
70+
string getConfigDesc() {
71+
exists(
72+
LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink, MethodAccess ma
73+
|
74+
this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
75+
cc.hasFlow(source, sink) and
76+
ma.getArgument(0) = sink.asExpr() and
77+
result = "Plaintext credentials " + this.getName() + " are loaded in " + ma
78+
)
79+
or
80+
not exists(LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink |
81+
this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
82+
cc.hasFlow(source, sink)
83+
) and
84+
result =
85+
"Plaintext credentials " + this.getName() + " have cleartext value " + this.getValue() +
86+
" in properties file"
87+
}
88+
}
89+
90+
/**
91+
* A dataflow configuration tracking flow of a method that loads a credentials property.
92+
*/
93+
class LoadCredentialsConfiguration extends DataFlow::Configuration {
94+
LoadCredentialsConfiguration() { this = "LoadCredentialsConfiguration" }
95+
96+
override predicate isSource(DataFlow::Node source) {
97+
exists(CredentialsConfig cc |
98+
source.asExpr().(CompileTimeConstantExpr).getStringValue() = cc.getName()
99+
)
100+
}
101+
102+
override predicate isSink(DataFlow::Node sink) {
103+
sink.asExpr() =
104+
any(MethodAccess ma |
105+
ma.getMethod()
106+
.getDeclaringType()
107+
.getASupertype*()
108+
.hasQualifiedName("java.util", "Properties") and
109+
ma.getMethod().getName() = "getProperty"
110+
).getArgument(0)
111+
}
78112
}
79113

80114
from CredentialsConfig cc
81-
where not isNotCleartextCredentials(cc.getValue())
82-
select cc,
83-
"Plaintext credentials " + cc.getName() + " have cleartext value " + cc.getValue() +
84-
" in properties file."
115+
select cc, cc.getConfigDesc()

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import java.io.IOException;
2+
import java.util.Properties;
3+
4+
public class PropertiesUtils {
5+
/* Properties declaration. */
6+
private static Properties properties;
7+
8+
/** Static block to initializing the properties. */
9+
static {
10+
properties = new Properties();
11+
try {
12+
properties.load(PropertiesUtils.class.getClassLoader().getResourceAsStream("configuration.properties"));
13+
} catch (IOException e) {
14+
e.printStackTrace();
15+
}
16+
}
17+
18+
/** Returns the LDAP DN property value. */
19+
public static String getLdapDN() {
20+
return properties.getProperty("ldap.loginDN");
21+
}
22+
23+
/** Returns the LDAP password property value. */
24+
public static String getLdapPassword() {
25+
return properties.getProperty("ldap.password");
26+
}
27+
28+
/** Returns the SQL Server username property value. */
29+
public static String getMSDataSourceUserName() {
30+
return properties.getProperty("datasource1.username");
31+
}
32+
33+
/** Returns the SQL Server password property value. */
34+
public static String getMSDataSourcePassword() {
35+
return properties.getProperty("datasource1.password");
36+
}
37+
38+
/** Returns the mail account property value. */
39+
public static String getMailUserName() {
40+
return properties.getProperty("mail.username");
41+
}
42+
43+
/** Returns the mail password property value. */
44+
public static String getMailPassword() {
45+
return properties.getProperty("mail.password");
46+
}
47+
48+
/** Returns the AWS Access Key property value. */
49+
public static String getAWSAccessKey() {
50+
return properties.getProperty("com.example.aws.s3.access_key");
51+
}
52+
53+
/** Returns the AWS Secret Key property value. */
54+
public static String getAWSSecretKey() {
55+
return properties.getProperty("com.example.aws.s3.secret_key");
56+
}
57+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
# GOOD: UI display messages; not credentials
12
prompt.username=Username
23
prompt.password=Password
34

0 commit comments

Comments
 (0)