Skip to content

Commit

Permalink
#271: don't encode filenames anymore, but do make sure all attachment…
Browse files Browse the repository at this point in the history
…/image file names are tested for injection attacks. Update the master demo
  • Loading branch information
bbottema committed Jul 12, 2020
1 parent 3540c7b commit 948555a
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ private static BodyPart getBodyPartFromDatasource(final AttachmentResource attac
throws MessagingException {
final BodyPart attachmentPart = new MimeBodyPart();
// setting headers isn't working nicely using the javax mail API, so let's do that manually
final String resourceName = determineResourceName(attachmentResource, false);
final String fileName = determineResourceName(attachmentResource, true);
final String resourceName = determineResourceName(attachmentResource, false, true);
final String fileName = determineResourceName(attachmentResource, true, false);
attachmentPart.setDataHandler(new DataHandler(new NamedDataSource(fileName, attachmentResource.getDataSource())));
attachmentPart.setFileName(fileName);
final String contentType = attachmentResource.getDataSource().getContentType();
Expand All @@ -248,7 +248,7 @@ private static BodyPart getBodyPartFromDatasource(final AttachmentResource attac
/**
* Determines the right resource name and optionally attaches the correct extension to the name. The result is mime encoded.
*/
static String determineResourceName(final AttachmentResource attachmentResource, final boolean includeExtension) {
static String determineResourceName(final AttachmentResource attachmentResource, final boolean includeExtension, final boolean encodeResourceName) {
final String datasourceName = attachmentResource.getDataSource().getName();

String resourceName;
Expand All @@ -265,7 +265,7 @@ static String determineResourceName(final AttachmentResource attachmentResource,
} else if (!includeExtension && resourceName.contains(".") && resourceName.equals(datasourceName)) {
resourceName = removeExtension(resourceName);
}
return MiscUtil.encodeText(resourceName);
return encodeResourceName ? MiscUtil.encodeText(resourceName) : resourceName;
}

@NotNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,11 @@ public static boolean validate(@NotNull final Email email, @NotNull final EnumSe
}
for (final AttachmentResource attachment : email.getAttachments()) {
scanForInjectionAttack(attachment.getName(), "email.attachment.name");
scanForInjectionAttack(attachment.getDataSource().getName(), "email.attachment.datasource.name");
}
for (final AttachmentResource embeddedImage : email.getEmbeddedImages()) {
scanForInjectionAttack(embeddedImage.getName(), "email.embeddedImage.name");
scanForInjectionAttack(embeddedImage.getDataSource().getName(), "email.embeddedImage.datasource.name");
}
scanForInjectionAttack(email.getFromRecipient().getName(), "email.fromRecipient.name");
scanForInjectionAttack(email.getFromRecipient().getAddress(), "email.fromRecipient.address");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import javax.mail.internet.MimeMessage;
import javax.mail.util.ByteArrayDataSource;
import java.io.IOException;
import java.nio.charset.Charset;
import static java.nio.charset.Charset.defaultCharset;

import static demo.ResourceFolderHelper.determineResourceFolder;

Expand Down Expand Up @@ -63,7 +63,8 @@ private static void testMixedRelatedAlternativeIncludingCalendarAndMessageParsin

// add two text files in different ways and a black thumbs up embedded image ->
emailPopulatingBuilderNormal.withAttachment("dresscode.txt", new ByteArrayDataSource("Black Tie Optional", "text/plain"));
emailPopulatingBuilderNormal.withAttachment("location.txt", "On the moon!".getBytes(Charset.defaultCharset()), "text/plain");
emailPopulatingBuilderNormal.withAttachment("location.txt", "On the moon!".getBytes(defaultCharset()), "text/plain");
emailPopulatingBuilderNormal.withAttachment("special_łąąśćńółęĄŻŹĆŃÓŁĘ.txt", "doorcode: Ken sent me".getBytes(defaultCharset()), "text/plain");
emailPopulatingBuilderNormal.withEmbeddedImage("thumbsup", produceThumbsUpImage(), "image/png");
emailPopulatingBuilderNormal.withCalendarText(CalendarMethod.REQUEST, CalendarHelper.createCalendarEvent());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,64 +45,64 @@ public void setup() {
public void determineResourceName1()
throws IOException {
AttachmentResource resource1 = new AttachmentResource(null, getDataSource("blahblah"));
assertThat(MimeMessageHelper.determineResourceName(resource1, false)).isEqualTo("blahblah");
assertThat(MimeMessageHelper.determineResourceName(resource1, true)).isEqualTo("blahblah");
assertThat(MimeMessageHelper.determineResourceName(resource1, false, true)).isEqualTo("blahblah");
assertThat(MimeMessageHelper.determineResourceName(resource1, true, true)).isEqualTo("blahblah");
}

@Test
public void determineResourceName2()
throws IOException {
AttachmentResource resource2 = new AttachmentResource(null, getDataSource("blahblah.txt"));
assertThat(MimeMessageHelper.determineResourceName(resource2, false)).isEqualTo("blahblah");
assertThat(MimeMessageHelper.determineResourceName(resource2, true)).isEqualTo("blahblah.txt");
assertThat(MimeMessageHelper.determineResourceName(resource2, false, true)).isEqualTo("blahblah");
assertThat(MimeMessageHelper.determineResourceName(resource2, true, true)).isEqualTo("blahblah.txt");
}

@Test
public void determineResourceName3()
throws IOException {
AttachmentResource resource3 = new AttachmentResource("the resource", getDataSource(null));
assertThat(MimeMessageHelper.determineResourceName(resource3, false)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource3, true)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource3, false, true)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource3, true, true)).isEqualTo("the resource");
}

@Test
public void determineResourceName4()
throws IOException {
AttachmentResource resource4 = new AttachmentResource("the resource", getDataSource("blahblah.txt"));
assertThat(MimeMessageHelper.determineResourceName(resource4, false)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource4, true)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource4, false, true)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource4, true, true)).isEqualTo("the resource.txt");
}

@Test
public void determineResourceName5()
throws IOException {
AttachmentResource resource5 = new AttachmentResource("the resource", getDataSource("blahblah"));
assertThat(MimeMessageHelper.determineResourceName(resource5, false)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource5, true)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource5, false, true)).isEqualTo("the resource");
assertThat(MimeMessageHelper.determineResourceName(resource5, true, true)).isEqualTo("the resource");
}

@Test
public void determineResourceName6()
throws IOException {
AttachmentResource resource6 = new AttachmentResource("the resource.txt", getDataSource("blahblah.txt"));
assertThat(MimeMessageHelper.determineResourceName(resource6, false)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource6, true)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource6, false, true)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource6, true, true)).isEqualTo("the resource.txt");
}

@Test
public void determineResourceName7()
throws IOException {
AttachmentResource resource7 = new AttachmentResource("the resource.txt", getDataSource("blahblah"));
assertThat(MimeMessageHelper.determineResourceName(resource7, false)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource7, true)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource7, false, true)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource7, true, true)).isEqualTo("the resource.txt");
}

@Test
public void determineResourceName_ignoreExtensionFromResource()
throws IOException {
AttachmentResource resource7 = new AttachmentResource("the resource.txt", getDataSource("blahblah.1/www/get?id=3"));
assertThat(MimeMessageHelper.determineResourceName(resource7, false)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource7, true)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource7, false, true)).isEqualTo("the resource.txt");
assertThat(MimeMessageHelper.determineResourceName(resource7, true, true)).isEqualTo("the resource.txt");
}

private ByteArrayDataSource getDataSource(@Nullable String name)
Expand Down

0 comments on commit 948555a

Please sign in to comment.