Skip to content

Commit 1349bf7

Browse files
luchua-bcluchua-bc
committed
Create a .qll file to reuse the code and add check of Spring properties
1 parent 5ce3f9d commit 1349bf7

File tree

8 files changed

+137
-199
lines changed

8 files changed

+137
-199
lines changed

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

Lines changed: 1 addition & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -18,41 +18,7 @@
1818
*/
1919

2020
import java
21-
import semmle.code.configfiles.ConfigFiles
22-
import semmle.code.java.dataflow.FlowSources
23-
24-
private string possibleSecretName() {
25-
result =
26-
[
27-
"%password%", "%passwd%", "%account%", "%accnt%", "%credential%", "%token%", "%secret%",
28-
"%access%key%"
29-
]
30-
}
31-
32-
private string possibleEncryptedSecretName() { result = ["%hashed%", "%encrypted%", "%crypt%"] }
33-
34-
/** Holds if the value is not cleartext credentials. */
35-
bindingset[value]
36-
predicate isNotCleartextCredentials(string value) {
37-
value = "" // Empty string
38-
or
39-
value.length() < 7 // Typical credentials are no less than 6 characters
40-
or
41-
value.matches("% %") // Sentences containing spaces
42-
or
43-
value.regexpMatch(".*[^a-zA-Z\\d]{3,}.*") // Contain repeated non-alphanumeric characters such as a fake password pass**** or ????
44-
or
45-
value.matches("@%") // Starts with the "@" sign
46-
or
47-
value.regexpMatch("\\$\\{.*\\}") // Variable placeholder ${credentials}
48-
or
49-
value.matches("%=") // A basic check of encrypted credentials ending with padding characters
50-
or
51-
value.matches("ENC(%)") // Encrypted value
52-
or
53-
// Could be a message property for UI display or fake passwords, e.g. login.password_expired=Your current password has expired.
54-
value.toLowerCase().matches(possibleSecretName())
55-
}
21+
import experimental.semmle.code.java.frameworks.CredentialsInPropertiesFile
5622

5723
/**
5824
* Holds if the credentials are in a non-production properties file indicated by:
@@ -63,63 +29,6 @@ predicate isNonProdCredentials(CredentialsConfig cc) {
6329
cc.getFile().getAbsolutePath().matches(["%dev%", "%test%", "%sample%"])
6430
}
6531

66-
/** The credentials configuration property. */
67-
class CredentialsConfig extends ConfigPair {
68-
CredentialsConfig() {
69-
this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
70-
not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName()) and
71-
not isNotCleartextCredentials(this.getValueElement().getValue().trim())
72-
}
73-
74-
string getName() { result = this.getNameElement().getName().trim() }
75-
76-
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-
}
121-
}
122-
12332
from CredentialsConfig cc
12433
where not isNonProdCredentials(cc)
12534
select cc, cc.getConfigDesc()

‎java/ql/src/experimental/semmle/code/java/frameworks/CredentialsInPropertiesFile.qll

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/**
2+
* Provides classes for analyzing properties files.
3+
*/
4+
5+
import java
6+
import semmle.code.configfiles.ConfigFiles
7+
import semmle.code.java.dataflow.FlowSources
8+
import semmle.code.java.frameworks.Properties
9+
10+
private string possibleSecretName() {
11+
result =
12+
[
13+
"%password%", "%passwd%", "%account%", "%accnt%", "%credential%", "%token%", "%secret%",
14+
"%access%key%"
15+
]
16+
}
17+
18+
private string possibleEncryptedSecretName() { result = ["%hashed%", "%encrypted%", "%crypt%"] }
19+
20+
/** Holds if the value is not cleartext credentials. */
21+
bindingset[value]
22+
predicate isNotCleartextCredentials(string value) {
23+
value = "" // Empty string
24+
or
25+
value.length() < 7 // Typical credentials are no less than 6 characters
26+
or
27+
value.matches("% %") // Sentences containing spaces
28+
or
29+
value.regexpMatch(".*[^a-zA-Z\\d]{3,}.*") // Contain repeated non-alphanumeric characters such as a fake password pass**** or ????
30+
or
31+
value.matches("@%") // Starts with the "@" sign
32+
or
33+
value.regexpMatch("\\$\\{.*\\}") // Variable placeholder ${credentials}
34+
or
35+
value.matches("%=") // A basic check of encrypted credentials ending with padding characters
36+
or
37+
value.matches("ENC(%)") // Encrypted value
38+
or
39+
// Could be a message property for UI display or fake passwords, e.g. login.password_expired=Your current password has expired.
40+
value.toLowerCase().matches(possibleSecretName())
41+
}
42+
43+
/** The credentials configuration property. */
44+
class CredentialsConfig extends ConfigPair {
45+
CredentialsConfig() {
46+
this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
47+
not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName()) and
48+
not isNotCleartextCredentials(this.getValueElement().getValue().trim())
49+
}
50+
51+
string getName() { result = this.getNameElement().getName().trim() }
52+
53+
string getValue() { result = this.getValueElement().getValue().trim() }
54+
55+
/** Returns a description of this vulnerability. */
56+
string getConfigDesc() {
57+
exists(
58+
// getProperty(...)
59+
LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink, MethodAccess ma
60+
|
61+
this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
62+
cc.hasFlow(source, sink) and
63+
ma.getArgument(0) = sink.asExpr() and
64+
result = "Plaintext credentials " + this.getName() + " are loaded in Java Properties " + ma
65+
)
66+
or
67+
exists(
68+
// @Value("${mail.password}")
69+
Annotation a
70+
|
71+
a.getType().hasQualifiedName("org.springframework.beans.factory.annotation", "Value") and
72+
a.getAValue().(CompileTimeConstantExpr).getStringValue() = "${" + this.getName() + "}" and
73+
result = "Plaintext credentials " + this.getName() + " are loaded in Spring annotation " + a
74+
)
75+
or
76+
not exists(LoadCredentialsConfiguration cc, DataFlow::Node source, DataFlow::Node sink |
77+
this.getName() = source.asExpr().(CompileTimeConstantExpr).getStringValue() and
78+
cc.hasFlow(source, sink)
79+
) and
80+
not exists(Annotation a |
81+
a.getType().hasQualifiedName("org.springframework.beans.factory.annotation", "Value") and
82+
a.getAValue().(CompileTimeConstantExpr).getStringValue() = "${" + this.getName() + "}"
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 cleartext credentials stored in a properties file
92+
* to a `Properties.getProperty(...)` method call.
93+
*/
94+
class LoadCredentialsConfiguration extends DataFlow::Configuration {
95+
LoadCredentialsConfiguration() { this = "LoadCredentialsConfiguration" }
96+
97+
override predicate isSource(DataFlow::Node source) {
98+
exists(CredentialsConfig cc |
99+
source.asExpr().(CompileTimeConstantExpr).getStringValue() = cc.getName()
100+
)
101+
}
102+
103+
override predicate isSink(DataFlow::Node sink) {
104+
sink.asExpr() =
105+
any(MethodAccess ma | ma.getMethod() instanceof PropertiesGetPropertyMethod).getArgument(0)
106+
}
107+
}

‎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 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(...) |
1+
| configuration.properties:6:1:6:25 | ldap.password=mysecpass | Plaintext credentials ldap.password are loaded in Java Properties getProperty(...) |
2+
| configuration.properties:18:1:18:35 | datasource1.password=Passw0rd@123 | Plaintext credentials datasource1.password are loaded in Java Properties getProperty(...) |
3+
| configuration.properties:25:1:25:31 | mail.password=MysecPWxWa@1993 | Plaintext credentials mail.password are loaded in Spring annotation Value |
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 Java Properties 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 Java Properties getProperty(...) |

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

Lines changed: 1 addition & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -18,98 +18,7 @@
1818
*/
1919

2020
import java
21-
import semmle.code.configfiles.ConfigFiles
22-
import semmle.code.java.dataflow.FlowSources
23-
24-
private string possibleSecretName() {
25-
result =
26-
[
27-
"%password%", "%passwd%", "%account%", "%accnt%", "%credential%", "%token%", "%secret%",
28-
"%access%key%"
29-
]
30-
}
31-
32-
private string possibleEncryptedSecretName() { result = ["%hashed%", "%encrypted%", "%crypt%"] }
33-
34-
/** Holds if the value is not cleartext credentials. */
35-
bindingset[value]
36-
predicate isNotCleartextCredentials(string value) {
37-
value = "" // Empty string
38-
or
39-
value.length() < 7 // Typical credentials are no less than 6 characters
40-
or
41-
value.matches("% %") // Sentences containing spaces
42-
or
43-
value.regexpMatch(".*[^a-zA-Z\\d]{3,}.*") // Contain repeated non-alphanumeric characters such as a fake password pass**** or ????
44-
or
45-
value.matches("@%") // Starts with the "@" sign
46-
or
47-
value.regexpMatch("\\$\\{.*\\}") // Variable placeholder ${credentials}
48-
or
49-
value.matches("%=") // A basic check of encrypted credentials ending with padding characters
50-
or
51-
value.matches("ENC(%)") // Encrypted value
52-
or
53-
// Could be a message property for UI display or fake passwords, e.g. login.password_expired=Your current password has expired.
54-
value.toLowerCase().matches(possibleSecretName())
55-
}
56-
57-
/** The credentials configuration property. */
58-
class CredentialsConfig extends ConfigPair {
59-
CredentialsConfig() {
60-
this.getNameElement().getName().trim().toLowerCase().matches(possibleSecretName()) and
61-
not this.getNameElement().getName().trim().toLowerCase().matches(possibleEncryptedSecretName()) and
62-
not isNotCleartextCredentials(this.getValueElement().getValue().trim())
63-
}
64-
65-
string getName() { result = this.getNameElement().getName().trim() }
66-
67-
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-
}
112-
}
21+
import experimental.semmle.code.java.frameworks.CredentialsInPropertiesFile
11322

11423
from CredentialsConfig cc
11524
select cc, cc.getConfigDesc()

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import org.springframework.beans.factory.annotation.Value;
2+
import org.springframework.context.annotation.Configuration;
3+
4+
@Configuration
5+
public class MailConfig {
6+
@Value("${mail.password}")
7+
private String mailPassword;
8+
9+
@Value("${mail.username}")
10+
private String mailUserName;
11+
}

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,6 @@ public static String getMSDataSourcePassword() {
3535
return properties.getProperty("datasource1.password");
3636
}
3737

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-
4838
/** Returns the AWS Access Key property value. */
4939
public static String getAWSAccessKey() {
5040
return properties.getProperty("com.example.aws.s3.access_key");

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3

‎java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/annotation/Value.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package org.springframework.beans.factory.annotation;
2+
3+
public @interface Value {
4+
5+
/**
6+
* The actual value expression such as <code>#{systemProperties.myProp}</code>
7+
* or property placeholder such as <code>${my.app.myProp}</code>.
8+
*/
9+
String value();
10+
11+
}

0 commit comments

Comments
 (0)