diff --git a/changes.xml b/changes.xml index b0cc5419..6f6678a0 100644 --- a/changes.xml +++ b/changes.xml @@ -23,6 +23,12 @@ xsi:schemaLocation="http://maven.apache.org/changes/1.0.0 http://maven.apache.org/plugins/maven-changes-plugin/xsd/changes-1.0.0.xsd"> + + + MediaFileServlet: Force Content-Disposition=attachment header for SVG files served by MediaFileServlet. Also ensure URLs to SVG images are always handled by MediaFileServlet. + + + Switch to AEM 6.5.7 as minimum version. diff --git a/src/main/java/io/wcm/handler/media/impl/AbstractMediaFileServlet.java b/src/main/java/io/wcm/handler/media/impl/AbstractMediaFileServlet.java index eec6b6fc..725370e9 100644 --- a/src/main/java/io/wcm/handler/media/impl/AbstractMediaFileServlet.java +++ b/src/main/java/io/wcm/handler/media/impl/AbstractMediaFileServlet.java @@ -158,22 +158,31 @@ protected void sendBinaryData(byte[] binaryData, String contentType, // Overwrite MIME type with one suited for downloads response.setContentType(ContentType.DOWNLOAD); - // Construct disposition header - StringBuilder dispositionHeader = new StringBuilder("attachment;"); - String suffix = request.getRequestPathInfo().getSuffix(); - suffix = StringUtils.stripStart(suffix, "/"); - if (StringUtils.isNotEmpty(suffix)) { - dispositionHeader.append("filename=\"").append(suffix).append('\"'); - } - - response.setHeader(HEADER_CONTENT_DISPOSITION, dispositionHeader.toString()); + // set content disposition header to file name from suffix + setContentDispositionAttachmentHeader(request, response); + } + + // special handling for SVG images which are not treated as download: + // force content disposition header to prevent stored XSS attack with malicious JavaScript in SVG file + else if (StringUtils.equals(contentType, ContentType.SVG)) { + setContentDispositionAttachmentHeader(request, response); } // write binary data OutputStream out = response.getOutputStream(); out.write(binaryData); out.flush(); + } + private void setContentDispositionAttachmentHeader(SlingHttpServletRequest request, SlingHttpServletResponse response) { + // Construct disposition header + StringBuilder dispositionHeader = new StringBuilder("attachment;"); + String suffix = request.getRequestPathInfo().getSuffix(); + suffix = StringUtils.stripStart(suffix, "/"); + if (StringUtils.isNotEmpty(suffix)) { + dispositionHeader.append("filename=\"").append(suffix).append('\"'); + } + response.setHeader(HEADER_CONTENT_DISPOSITION, dispositionHeader.toString()); } } diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/RenditionMetadata.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/RenditionMetadata.java index 3d747070..d8624429 100644 --- a/src/main/java/io/wcm/handler/mediasource/dam/impl/RenditionMetadata.java +++ b/src/main/java/io/wcm/handler/mediasource/dam/impl/RenditionMetadata.java @@ -177,6 +177,10 @@ public void setMediaFormat(MediaFormat mediaFormat) { + "." + MediaFileServlet.SELECTOR_DOWNLOAD + "." + MediaFileServlet.EXTENSION, getFileName(contentDispositionAttachment)); } + else if (MediaFileType.isVectorImage(getFileExtension())) { + return RenditionMetadata.buildMediaPath(getRendition().getPath() + "." + MediaFileServlet.SELECTOR + + "." + MediaFileServlet.EXTENSION, getFileName(contentDispositionAttachment)); + } else if (MediaFileType.isBrowserImage(getFileExtension()) || !MediaFileType.isImage(getFileExtension())) { // use "deep URL" to reference rendition directly // do not use Asset URL for original rendition because it creates conflicts for dispatcher cache (filename vs. directory for asset resource name) diff --git a/src/test/java/io/wcm/handler/media/impl/MediaFileServletTest.java b/src/test/java/io/wcm/handler/media/impl/MediaFileServletTest.java index 07232f58..b683749c 100644 --- a/src/test/java/io/wcm/handler/media/impl/MediaFileServletTest.java +++ b/src/test/java/io/wcm/handler/media/impl/MediaFileServletTest.java @@ -20,6 +20,7 @@ package io.wcm.handler.media.impl; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import javax.servlet.http.HttpServletResponse; @@ -37,6 +38,7 @@ class MediaFileServletTest { private static final long EXPECTED_CONTENT_LENGTH = 15471; + private static final long EXPECTED_CONTENT_LENGTH_SVG = 718; private final AemContext context = AppAemContext.newAemContext(); @@ -56,6 +58,21 @@ void testGet() throws Exception { assertEquals(ContentType.JPEG, context.response().getContentType()); assertEquals(EXPECTED_CONTENT_LENGTH, context.response().getOutput().length); assertEquals(EXPECTED_CONTENT_LENGTH, context.response().getContentLength()); + assertNull(context.response().getHeader(AbstractMediaFileServlet.HEADER_CONTENT_DISPOSITION)); + } + + @Test + void testGet_SVG() throws Exception { + context.currentResource(context.load().binaryFile("/filetype/sample.svg", "/content/sample.svg")); + + underTest.service(context.request(), context.response()); + + assertEquals(HttpServletResponse.SC_OK, context.response().getStatus()); + assertEquals(ContentType.SVG, context.response().getContentType()); + assertEquals(EXPECTED_CONTENT_LENGTH_SVG, context.response().getOutput().length); + assertEquals(EXPECTED_CONTENT_LENGTH_SVG, context.response().getContentLength()); + // forced content disposition header for SVG to prevent stored XSS + assertEquals("attachment;", context.response().getHeader(AbstractMediaFileServlet.HEADER_CONTENT_DISPOSITION)); } @Test diff --git a/src/test/java/io/wcm/handler/media/impl/MediaHandlerImplImageFileTypesEnd2EndTest.java b/src/test/java/io/wcm/handler/media/impl/MediaHandlerImplImageFileTypesEnd2EndTest.java index 299e518f..b7b556db 100644 --- a/src/test/java/io/wcm/handler/media/impl/MediaHandlerImplImageFileTypesEnd2EndTest.java +++ b/src/test/java/io/wcm/handler/media/impl/MediaHandlerImplImageFileTypesEnd2EndTest.java @@ -318,7 +318,7 @@ void testFileUpload_TIFF_AutoCrop() { void testAsset_SVG_Original() { Asset asset = createSampleAsset("/filetype/sample.svg", ContentType.SVG); buildAssertMedia(asset, 100, 50, - "/content/dam/sample.svg/_jcr_content/renditions/original./sample.svg", + "/content/dam/sample.svg/_jcr_content/renditions/original.media_file.file/sample.svg", ContentType.SVG); } @@ -334,7 +334,7 @@ void testAsset_SVG_Original_ContentDisposition() { void testAsset_SVG_Rescale() { Asset asset = createSampleAsset("/filetype/sample.svg", ContentType.SVG); buildAssertMedia_Rescale(asset, 80, 40, - "/content/dam/sample.svg/_jcr_content/renditions/original./sample.svg", + "/content/dam/sample.svg/_jcr_content/renditions/original.media_file.file/sample.svg", ContentType.SVG); }