Skip to content

Commit d0b307e

Browse files
authored
Merge pull request github#6103 from atorralba/atorralba/promote-insecure-javamail
Java: Promote Insecure JavaMail SSL Configuration from experimental
2 parents 9505846 + a86cbd8 commit d0b307e

File tree

22 files changed

+463
-192
lines changed

22 files changed

+463
-192
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* The query "Insecure JavaMail SSL Configuration" (`java/insecure-smtp-ssl`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/3491).
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/** Provides classes and predicates to work with email */
2+
3+
import java
4+
5+
/**
6+
* The class `javax.mail.Session` or `jakarta.mail.Session`.
7+
*/
8+
class MailSession extends Class {
9+
MailSession() { this.hasQualifiedName(["javax.mail", "jakarta.mail"], "Session") }
10+
}
11+
12+
/**
13+
* The method `getInstance` of the classes `javax.mail.Session` or `jakarta.mail.Session`.
14+
*/
15+
class MailSessionGetInstanceMethod extends Method {
16+
MailSessionGetInstanceMethod() {
17+
this.getDeclaringType() instanceof MailSession and
18+
this.getName() = "getInstance"
19+
}
20+
}
21+
22+
/**
23+
* A subtype of the class `org.apache.commons.mail.Email`.
24+
*/
25+
class ApacheEmail extends Class {
26+
ApacheEmail() { this.getASupertype*().hasQualifiedName("org.apache.commons.mail", "Email") }
27+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/** Provides classes and predicates to reason about email vulnerabilities. */
2+
3+
import java
4+
import semmle.code.java.frameworks.Mail
5+
private import semmle.code.java.frameworks.Properties
6+
7+
/**
8+
* The insecure way to set Java properties in mail sessions.
9+
* 1. Set the `mail.smtp.auth` property to provide the SMTP Transport with a username and password when connecting to the SMTP server or
10+
* set the `mail.smtp.ssl.socketFactory`/`mail.smtp.ssl.socketFactory.class` property to create an SMTP SSL socket.
11+
* 2. No `mail.smtp.ssl.checkserveridentity` property is enabled.
12+
*/
13+
predicate isInsecureMailPropertyConfig(Variable properties) {
14+
exists(MethodAccess ma |
15+
ma.getMethod() instanceof SetPropertyMethod and
16+
ma.getQualifier() = properties.getAnAccess()
17+
|
18+
getStringValue(ma.getArgument(0)).matches("%.auth%") and //mail.smtp.auth
19+
getStringValue(ma.getArgument(1)) = "true"
20+
or
21+
getStringValue(ma.getArgument(0)).matches("%.socketFactory%") //mail.smtp.socketFactory or mail.smtp.socketFactory.class
22+
) and
23+
not exists(MethodAccess ma |
24+
ma.getMethod() instanceof SetPropertyMethod and
25+
ma.getQualifier() = properties.getAnAccess()
26+
|
27+
getStringValue(ma.getArgument(0)).matches("%.ssl.checkserveridentity%") and //mail.smtp.ssl.checkserveridentity
28+
getStringValue(ma.getArgument(1)) = "true"
29+
)
30+
}
31+
32+
/**
33+
* Holds if `ma` enables TLS/SSL with Apache Email.
34+
*/
35+
predicate enablesEmailSsl(MethodAccess ma) {
36+
ma.getMethod().hasName(["setSSLOnConnect", "setStartTLSRequired"]) and
37+
ma.getMethod().getDeclaringType() instanceof ApacheEmail and
38+
ma.getArgument(0).(BooleanLiteral).getBooleanValue() = true
39+
}
40+
41+
/**
42+
* Holds if a SSL certificate check is enabled on an access of `apacheEmail` with Apache Email.
43+
*/
44+
predicate hasSslCertificateCheck(Variable apacheEmail) {
45+
exists(MethodAccess ma |
46+
ma.getQualifier() = apacheEmail.getAnAccess() and
47+
ma.getMethod().hasName("setSSLCheckServerIdentity") and
48+
ma.getMethod().getDeclaringType() instanceof ApacheEmail and
49+
ma.getArgument(0).(BooleanLiteral).getBooleanValue() = true
50+
)
51+
}
52+
53+
/**
54+
* Returns the string value of `expr` if it is a `CompileTimeConstantExpr`,
55+
* or the string value of its operands if it is an `AddExpr`.
56+
*/
57+
private string getStringValue(Expr expr) {
58+
result = expr.(CompileTimeConstantExpr).getStringValue()
59+
or
60+
result = getStringValue(expr.(AddExpr).getAnOperand())
61+
}
62+
63+
/**
64+
* A method to set Java properties, either using the `Properties` class
65+
* or the `Dictionary` class.
66+
*/
67+
private class SetPropertyMethod extends Method {
68+
SetPropertyMethod() {
69+
this instanceof PropertiesSetPropertyMethod
70+
or
71+
this.hasName("put") and
72+
this.getDeclaringType().getASourceSupertype*().hasQualifiedName("java.util", "Dictionary")
73+
}
74+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>JavaMail is commonly used in Java applications to send emails. There are popular third-party libraries like Apache Commons Email which are built on JavaMail and facilitate integration. Authenticated mail sessions require user credentials and mail sessions can require SSL/TLS authentication. It is a common security vulnerability that host-specific certificate data is not validated or is incorrectly validated. Failing to validate the certificate makes the SSL session susceptible to a man-in-the-middle attack.</p>
6+
<p>This query checks whether the SSL certificate is validated when credentials are used and SSL is enabled in email communications.</p>
7+
<p>The query has code for both plain JavaMail invocation and mailing through Apache SimpleMail to make it more comprehensive.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Validate SSL certificate when sensitive information is sent in email communications.</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>The following two examples show two ways of configuring secure emails through JavaMail or Apache SimpleMail. In the 'BAD' case,
16+
credentials are sent in an SSL session without certificate validation. In the 'GOOD' case, the certificate is validated.</p>
17+
<sample src="JavaMail.java" />
18+
<sample src="SimpleMail.java" />
19+
</example>
20+
21+
<references>
22+
<li>
23+
Jakarta Mail:
24+
<a href="https://eclipse-ee4j.github.io/mail/docs/SSLNOTES.txt">SSL Notes</a>.
25+
</li>
26+
<li>
27+
Apache Commons:
28+
<a href="https://commons.apache.org/proper/commons-email/userguide.html#Security">Email security</a>.
29+
</li>
30+
<li>
31+
Log4j2:
32+
<a href="https://issues.apache.org/jira/browse/LOG4J2-2819">Add support for specifying an SSL configuration for SmtpAppender (CVE-2020-9488)</a>.
33+
</li>
34+
</references>
35+
</qhelp>
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Insecure JavaMail SSL Configuration
3+
* @description Configuring a Java application to use authenticated mail session
4+
* over SSL without certificate validation
5+
* makes the session susceptible to a man-in-the-middle attack.
6+
* @kind problem
7+
* @problem.severity warning
8+
* @security-severity 5.9
9+
* @precision medium
10+
* @id java/insecure-smtp-ssl
11+
* @tags security
12+
* external/cwe/cwe-297
13+
*/
14+
15+
import java
16+
import semmle.code.java.security.Mail
17+
18+
from MethodAccess ma
19+
where
20+
ma.getMethod() instanceof MailSessionGetInstanceMethod and
21+
isInsecureMailPropertyConfig(ma.getArgument(0).(VarAccess).getVariable())
22+
or
23+
enablesEmailSsl(ma) and not hasSslCertificateCheck(ma.getQualifier().(VarAccess).getVariable())
24+
select ma, "Java mailing has insecure SSL configuration"

java/ql/src/experimental/Security/CWE/CWE-297/InsecureJavaMail.qhelp

Lines changed: 0 additions & 36 deletions
This file was deleted.

java/ql/src/experimental/Security/CWE/CWE-297/InsecureJavaMail.ql

Lines changed: 0 additions & 107 deletions
This file was deleted.

java/ql/test/experimental/query-tests/security/CWE-297/InsecureJavaMail.expected

Lines changed: 0 additions & 2 deletions
This file was deleted.

0 commit comments

Comments
 (0)