Skip to content

Commit

Permalink
#171: fixed bug where "References" header raised a CRLF Injection fla…
Browse files Browse the repository at this point in the history
…g. Also added test to verify the "References" header is filled properly by replying multiple times in a thread.
  • Loading branch information
bbottema committed May 22, 2019
1 parent f824285 commit 80ec690
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 8 deletions.
12 changes: 11 additions & 1 deletion src/main/java/org/simplejavamail/mailer/Mailer.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import javax.mail.PasswordAuthentication;
import javax.mail.Session;
import javax.mail.internet.MimeMessage;
import javax.mail.internet.MimeUtility;
import java.util.EnumSet;
import java.util.Map;
import java.util.Properties;
Expand Down Expand Up @@ -302,7 +303,11 @@ public boolean validate(final Email email)
scanForInjectionAttack(email.getSubject(), "email.subject");
for (final Map.Entry<String, String> headerEntry : email.getHeaders().entrySet()) {
scanForInjectionAttack(headerEntry.getKey(), "email.header.mapEntryKey");
scanForInjectionAttack(headerEntry.getValue(), "email.header." + headerEntry.getKey());
if (headerEntry.getKey().equals("References")) {
scanForInjectionAttack(MimeUtility.unfold(headerEntry.getValue()), "email.header.References");
} else {
scanForInjectionAttack(headerEntry.getValue(), "email.header." + headerEntry.getKey());
}
}
for (final AttachmentResource attachment : email.getAttachments()) {
scanForInjectionAttack(attachment.getName(), "email.attachment.name");
Expand Down Expand Up @@ -331,6 +336,11 @@ public boolean validate(final Email email)
/**
* @param value Value checked for suspicious newline characters "\n", "\r" and "%0A" (as acknowledged by SMTP servers).
* @param valueLabel The name of the field being checked, used for reporting exceptions.
*
* @see <a href="http://www.cakesolutions.net/teamblogs/2008/05/08/email-header-injection-security">http://www.cakesolutions.net/teamblogs/2008/05/08/email-header-injection-security</a>
* @see <a href="https://security.stackexchange.com/a/54100/110048">https://security.stackexchange.com/a/54100/110048</a>
* @see <a href="https://www.owasp.org/index.php/Testing_for_IMAP/SMTP_Injection_(OTG-INPVAL-011)">https://www.owasp.org/index.php/Testing_for_IMAP/SMTP_Injection_(OTG-INPVAL-011)</a>
* @see <a href="http://cwe.mitre.org/data/definitions/93.html">http://cwe.mitre.org/data/definitions/93.html</a>
*/
private static void scanForInjectionAttack(final String value, final String valueLabel) {
if (value != null && (value.contains("\n") || value.contains("\r") || value.contains("%0A"))) {
Expand Down
38 changes: 31 additions & 7 deletions src/test/java/org/simplejavamail/mailer/MailerLiveTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@

import javax.mail.MessagingException;
import javax.mail.internet.MimeMessage;
import javax.mail.internet.MimeUtility;
import java.io.IOException;
import java.util.Properties;

import static java.lang.String.format;
import static javax.mail.Message.RecipientType.TO;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.data.MapEntry.entry;
Expand Down Expand Up @@ -138,7 +140,7 @@ private Email assertSendingEmail(final EmailPopulatingBuilder originalEmailPopul

@Test
public void createMailSession_ReplyToMessage()
throws IOException, MessagingException {
throws MessagingException {
// send initial mail
mailer.sendMail(readOutlookMessage("test-messages/HTML mail with replyto and attachment and embedded image.msg").buildEmail());
MimeMessage receivedMimeMessage = smtpServerRule.getOnlyMessage();
Expand Down Expand Up @@ -170,7 +172,7 @@ public void createMailSession_ReplyToMessage()

@Test
public void createMailSession_ReplyToMessage_NotAll_AndCustomReferences()
throws IOException, MessagingException {
throws MessagingException {
// send initial mail
mailer.sendMail(readOutlookMessage("test-messages/HTML mail with replyto and attachment and embedded image.msg").buildEmail());
MimeMessage receivedMimeMessage = smtpServerRule.getOnlyMessage();
Expand All @@ -179,20 +181,42 @@ public void createMailSession_ReplyToMessage_NotAll_AndCustomReferences()
// send reply to initial mail
Email reply = EmailBuilder
.replyingTo(assertSendingEmail(receivedEmailPopulatingBuilder, false))
.withHeader("References", "dummy-references")
.from("dummy@domain.com")
.from("Moo Shmoo", "dummy@domain.com")
.withPlainText("This is the reply")
.buildEmail();

// test received reply to initial mail
mailer.sendMail(reply);
MimeMessage receivedMimeMessageReply1 = smtpServerRule.getOnlyMessage("lo.pop.replyto@somemail.com");
Email receivedReply = mimeMessageToEmail(receivedMimeMessageReply1);
MimeMessage receivedMimeMessageReply = smtpServerRule.getOnlyMessage("lo.pop.replyto@somemail.com");
Email receivedReply = mimeMessageToEmail(receivedMimeMessageReply);

EmailAssert.assertThat(receivedReply).hasSubject("Re: hey");
EmailAssert.assertThat(receivedReply).hasOnlyRecipients(new Recipient("lollypop-replyto", "lo.pop.replyto@somemail.com", TO));
assertThat(receivedReply.getHeaders()).contains(entry("In-Reply-To", receivedEmailPopulatingBuilder.getId()));
assertThat(receivedReply.getHeaders()).contains(entry("References", "dummy-references"));
assertThat(receivedReply.getHeaders()).contains(entry("References", receivedEmailPopulatingBuilder.getId()));

EmailPopulatingBuilder receivedEmailReplyPopulatingBuilder = mimeMessageToEmailBuilder(receivedMimeMessageReply);

Email replyToReply = EmailBuilder
.replyingTo(assertSendingEmail(receivedEmailReplyPopulatingBuilder, false))
.from("Pappa Moo", "dummy@domain.com")
.withPlainText("This is the reply to the reply")
.buildEmail();

// test received reply to initial mail
mailer.sendMail(replyToReply);
MimeMessage receivedMimeMessageReplyToReply = smtpServerRule.getOnlyMessage("dummy@domain.com");
Email receivedReplyToReply = mimeMessageToEmail(receivedMimeMessageReplyToReply);

EmailAssert.assertThat(receivedReplyToReply).hasSubject("Re: hey");
EmailAssert.assertThat(receivedReplyToReply).hasOnlyRecipients(new Recipient("Moo Shmoo", "dummy@domain.com", TO));
assertThat(receivedReplyToReply.getHeaders()).contains(entry("In-Reply-To", receivedEmailReplyPopulatingBuilder.getId()));

assertThat(receivedReplyToReply.getHeaders()).contains(entry("References",
MimeUtility.fold("References: ".length(), format("%s\n%s",
receivedEmailPopulatingBuilder.getId(),
receivedEmailReplyPopulatingBuilder.getId()))
));
}

private void assertAttachmentMetadata(AttachmentResource embeddedImg, String mimeType, String filename) {
Expand Down

0 comments on commit 80ec690

Please sign in to comment.