Fix string/type match and add a test case

Author: luchua-bc

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

@@ -33,17 +33,17 @@ predicate isInsecureMailPropertyConfig(VarAccess propertiesVarAccess) {
     ma.getMethod() instanceof SetPropertyMethod and
     ma.getQualifier() = propertiesVarAccess.getVariable().getAnAccess() and
     (
-      getStringValue(ma.getArgument(0)).indexOf(".auth") != -1 and //mail.smtp.auth
+      getStringValue(ma.getArgument(0)).matches("%.auth%") and //mail.smtp.auth
       getStringValue(ma.getArgument(1)) = "true"
       or
-      getStringValue(ma.getArgument(0)).indexOf(".socketFactory") != -1 //mail.smtp.socketFactory or mail.smtp.socketFactory.class
+      getStringValue(ma.getArgument(0)).matches("%.socketFactory%") //mail.smtp.socketFactory or mail.smtp.socketFactory.class
     )
   ) and
   not exists(MethodAccess ma |
     ma.getMethod() instanceof SetPropertyMethod and
     ma.getQualifier() = propertiesVarAccess.getVariable().getAnAccess() and
     (
-      getStringValue(ma.getArgument(0)).indexOf(".ssl.checkserveridentity") != -1 and //mail.smtp.ssl.checkserveridentity
+      getStringValue(ma.getArgument(0)).matches("%.ssl.checkserveridentity%") and //mail.smtp.ssl.checkserveridentity
       getStringValue(ma.getArgument(1)) = "true"
     )
   )
@@ -53,11 +53,7 @@ predicate isInsecureMailPropertyConfig(VarAccess propertiesVarAccess) {
  * Helper method to get string value of an argument
  */
 string getStringValue(Expr expr) {
-  result = expr.(StringLiteral).getRepresentedString()
-  or
-  exists(Variable v | expr = v.getAnAccess() |
-    result = getStringValue(v.getInitializer().(CompileTimeConstantExpr))
-  )
+  result = expr.(CompileTimeConstantExpr).getStringValue()
   or
   result = getStringValue(expr.(AddExpr).getLeftOperand())
   or
@@ -68,14 +64,14 @@ string getStringValue(Expr expr) {
  * The JavaMail session class `javax.mail.Session`
  */
 class MailSession extends RefType {
-  MailSession() { this.getQualifiedName() = "javax.mail.Session" }
+  MailSession() { this.hasQualifiedName("javax.mail", "Session") }
 }
 
 /**
  * The class of Apache SimpleMail
  */
 class SimpleMail extends RefType {
-  SimpleMail() { this.getQualifiedName() = "org.apache.commons.mail.SimpleEmail" }
+  SimpleMail() { this.hasQualifiedName("org.apache.commons.mail", "SimpleEmail") }
 }
 
 /**
@@ -101,7 +97,7 @@ from MethodAccess ma
 where
   ma.getMethod().getDeclaringType() instanceof MailSession and
   ma.getMethod().getName() = "getInstance" and
-  isInsecureMailPropertyConfig(ma.getArgument(0).(VarAccess))
+  isInsecureMailPropertyConfig(ma.getArgument(0))
   or
-  enableTLSWithSimpleMail(ma) and hasNoCertCheckWithSimpleMail(ma.getQualifier().(VarAccess))
-select ma, "Java mailing has insecure SSL configuration"
+  enableTLSWithSimpleMail(ma) and hasNoCertCheckWithSimpleMail(ma.getQualifier())
+select ma, "Java mailing has insecure SSL configuration"

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

@@ -0,0 +1,2 @@
+| InsecureJavaMail.java:33:28:33:73 | getInstance(...) | Java mailing has insecure SSL configuration |
+| InsecureJavaMail.java:41:3:41:29 | setSSLOnConnect(...) | Java mailing has insecure SSL configuration |

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

@@ -0,0 +1,49 @@
+import java.util.Properties;
+
+import javax.activation.DataSource;
+import javax.mail.Authenticator;
+import javax.mail.Message;
+import javax.mail.MessagingException;
+import javax.mail.PasswordAuthentication;
+import javax.mail.Session;
+
+import org.apache.commons.mail.DefaultAuthenticator;
+import org.apache.commons.mail.Email;
+import org.apache.commons.mail.EmailException;
+import org.apache.commons.mail.SimpleEmail;
+
+import java.util.Properties;
+
+class InsecureJavaMail {
+	public void testJavaMail() {
+			final Properties properties = new Properties();
+			properties.put("mail.transport.protocol", "protocol");
+			properties.put("mail.smtp.host", "hostname");
+			properties.put("mail.smtp.socketFactory.class", "classname");
+
+			final javax.mail.Authenticator authenticator = new javax.mail.Authenticator() {
+					protected PasswordAuthentication getPasswordAuthentication() {
+							return new PasswordAuthentication("username", "password");
+					}
+			};
+			if (null != authenticator) {
+					properties.put("mail.smtp.auth", "true");
+					// properties.put("mail.smtp.ssl.checkserveridentity", "true");
+			}
+			final Session session = Session.getInstance(properties, authenticator);
+	}
+
+	public void testSimpleMail() {
+		Email email = new SimpleEmail();
+		email.setHostName("config.hostName");
+		email.setSmtpPort(25);
+		email.setAuthenticator(new DefaultAuthenticator("config.username", "config.password"));
+		email.setSSLOnConnect(true);
+		// email.setSSLCheckServerIdentity(true);
+		email.setFrom("fromAddress");
+		email.setSubject("subject");
+		email.setMsg("body");
+		email.addTo("toAddress");
+		email.send();
+	}
+}

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

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-297/InsecureJavaMail.ql