diff --git a/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java b/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java index 7906990fd..1416e02f2 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java @@ -22,6 +22,7 @@ import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; +import java.io.InputStream; import java.util.Date; import java.util.List; import org.apache.roller.weblogger.pojos.MediaFile; @@ -159,12 +160,57 @@ public ThemeResource getResource(String path) { try { MediaFileManager mmgr = WebloggerFactory.getWeblogger().getMediaFileManager(); - MediaFile mf = mmgr.getMediaFileByOriginalPath( - this.weblog, path); + MediaFile mediaFile = mmgr.getMediaFileByOriginalPath(this.weblog, path); + if (mediaFile != null) { + resource = new MediaFileThemeResource(mediaFile); + } } catch (WebloggerException ex) { // ignored, resource considered not found } return resource; } + + private static final class MediaFileThemeResource implements ThemeResource { + private final MediaFile mediaFile; + + private MediaFileThemeResource(MediaFile mediaFile) { + this.mediaFile = mediaFile; + } + + @Override + public String getName() { + return mediaFile.getName(); + } + + @Override + public String getPath() { + return mediaFile.getOriginalPath(); + } + + @Override + public long getLastModified() { + return mediaFile.getLastModified(); + } + + @Override + public long getLength() { + return mediaFile.getLength(); + } + + @Override + public InputStream getInputStream() { + return mediaFile.getInputStream(); + } + + @Override + public boolean isDirectory() { + return false; + } + + @Override + public int compareTo(ThemeResource other) { + return getPath().compareTo(other.getPath()); + } + } } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java index 713e4c2d1..a42faec28 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java @@ -31,6 +31,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.util.RollerConstants; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -116,7 +117,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) // set the content type based on whatever is in our web.xml mime defs if (resourceRequest.isThumbnail()) { - response.setContentType("image/png"); + MediaTypePolicy.applyResponseHeaders(response, "image/png", + mediaFile.getName()); try { resourceStream = mediaFile.getThumbnailInputStream(); } catch (Exception e) { @@ -131,7 +133,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } if (resourceStream == null) { - response.setContentType(mediaFile.getContentType()); + MediaTypePolicy.applyResponseHeaders(response, + mediaFile.getContentType(), mediaFile.getName()); resourceStream = mediaFile.getInputStream(); } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java index 7103bea9f..3fea57ff2 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java @@ -29,6 +29,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -129,7 +130,9 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // if not from theme then see if resource is in weblog's upload dir + boolean fromUploadedMedia = false; if (resourceStream == null) { + fromUploadedMedia = true; try { MediaFileManager mmgr = WebloggerFactory.getWeblogger() .getMediaFileManager(); @@ -160,8 +163,19 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // set the content type based on whatever is in our web.xml mime defs - response.setContentType(this.context.getMimeType(resourceRequest - .getResourcePath())); + String resourceType = MediaTypePolicy.typeFromName( + resourceRequest.getResourcePath(), this.context::getMimeType); + if (fromUploadedMedia) { + // Uploaded through the media library, so it is governed by the + // same policy as any other media response. + MediaTypePolicy.applyResponseHeaders(response, resourceType, + resourceRequest.getResourcePath()); + } else { + // A theme resource: authored as part of the theme and served as + // the type the theme intends, but never re-typed by the browser. + response.setHeader("X-Content-Type-Options", "nosniff"); + response.setContentType(resourceType); + } try { // ok, lets serve up the file diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java index 8dbd5dba1..4f793e703 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -125,7 +126,9 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // if not from theme then see if resource is in weblog's upload dir + boolean fromUploadedMedia = false; if (resourceStream == null) { + fromUploadedMedia = true; try { MediaFileManager mmgr = WebloggerFactory.getWeblogger() .getMediaFileManager(); @@ -159,8 +162,19 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // set the content type based on whatever is in our web.xml mime defs - response.setContentType(this.context.getMimeType(resourceRequest - .getResourcePath())); + String resourceType = MediaTypePolicy.typeFromName( + resourceRequest.getResourcePath(), this.context::getMimeType); + if (fromUploadedMedia) { + // Uploaded through the media library, so it is governed by the + // same policy as any other media response. + MediaTypePolicy.applyResponseHeaders(response, resourceType, + resourceRequest.getResourcePath()); + } else { + // A theme resource: authored as part of the theme and served as + // the type the theme intends, but never re-typed by the browser. + response.setHeader("X-Content-Type-Options", "nosniff"); + response.setContentType(resourceType); + } try { // ok, lets serve up the file diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java index 881dad754..bee4996e4 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java @@ -33,8 +33,10 @@ import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; import org.apache.roller.weblogger.pojos.MediaFile; import org.apache.roller.weblogger.pojos.MediaFileDirectory; +import org.apache.roller.weblogger.ui.core.RollerContext; import org.apache.roller.weblogger.util.RollerMessages; import org.apache.roller.weblogger.util.RollerMessages.RollerMessage; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.util.Utilities; import org.apache.struts2.interceptor.validation.SkipValidation; @@ -173,23 +175,19 @@ public String save() { mediaFile.setLength(this.uploadedFiles[i].length()); mediaFile.setInputStream(new FileInputStream( this.uploadedFiles[i])); - mediaFile - .setContentType(this.uploadedFilesContentType[i]); - - // in some cases Struts2 is not able to guess the content - // type correctly and assigns the default, which is - // octet-stream. So in cases where we see octet-stream - // we double check and see if we can guess the content - // type via the Java MIME type facilities. - mediaFile.setContentType(this.uploadedFilesContentType[i]); - if (mediaFile.getContentType() == null - || mediaFile.getContentType().endsWith("/octet-stream")) { - - String ctype = Utilities.getContentTypeFromFileName(mediaFile.getName()); - if (null != ctype) { - mediaFile.setContentType(ctype); - } + // The type the browser put on the part describes what + // the sender meant to send. It is taken as a hint and + // the stored type is worked out from the file name. + String declaredType = MediaTypePolicy.normalizeType( + this.uploadedFilesContentType[i]); + if (!WebloggerFactory.getWeblogger().getFileContentManager().canSave( + getActionWeblog(), fileName, declaredType, + this.uploadedFiles[i].length(), errors)) { + continue; } + mediaFile.setContentType(MediaTypePolicy.storedTypeFor( + mediaFile.getName(), declaredType, + RollerContext.getServletContext()::getMimeType)); manager.createMediaFile(getActionWeblog(), mediaFile, errors); WebloggerFactory.getWeblogger().flush(); diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java index 76ff2dbb7..fa089b6c0 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java @@ -23,12 +23,15 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.FileIOException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; import org.apache.roller.weblogger.pojos.MediaFile; import org.apache.roller.weblogger.pojos.MediaFileDirectory; +import org.apache.roller.weblogger.ui.core.RollerContext; +import org.apache.roller.weblogger.util.RollerMessages; import org.apache.struts2.convention.annotation.AllowedMethods; import org.apache.struts2.interceptor.validation.SkipValidation; @@ -123,8 +126,20 @@ public String save() { bean.copyTo(mediaFile); if (uploadedFile != null) { - mediaFile.setLength(this.uploadedFile.length()); - mediaFile.setContentType(this.uploadedFileContentType); + mediaFile.setLength(this.uploadedFile.length()); + String declaredType = MediaTypePolicy.normalizeType( + this.uploadedFileContentType); + RollerMessages errors = new RollerMessages(); + if (!WebloggerFactory.getWeblogger().getFileContentManager().canSave( + getActionWeblog(), this.uploadedFileName, declaredType, + this.uploadedFile.length(), errors)) { + throw new FileIOException(errors.toString()); + } + // Replacing the body re-decides the type, on the same + // terms as the original upload. + mediaFile.setContentType(MediaTypePolicy.storedTypeFor( + this.uploadedFileName, declaredType, + RollerContext.getServletContext()::getMimeType)); manager.updateMediaFile(getActionWeblog(), mediaFile, new FileInputStream(this.uploadedFile)); } else { diff --git a/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java b/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java new file mode 100644 index 000000000..2c8a7ebee --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java @@ -0,0 +1,203 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. For additional + * information regarding copyright in this work, please see the NOTICE + * file in the top level directory of this distribution. + */ + +package org.apache.roller.weblogger.util; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Locale; +import java.util.Set; + +import javax.servlet.http.HttpServletResponse; + +/** + * Decides what type an uploaded file is stored as, and how it is served back. + * + *

A client uploading a file states a type, but the stored type is derived + * from the file name. The declared value is a hint only, consulted where the + * name yields nothing, and it cannot introduce a type the browser would + * execute. + * + *

Serving applies the second half. Only a short list of formats that + * browsers render passively are sent inline; everything else is sent as an + * attachment, and {@code nosniff} accompanies every response so browsers do + * not substitute their own type guess. + */ +public final class MediaTypePolicy { + + private MediaTypePolicy() { + } + + public static final String DEFAULT_TYPE = "application/octet-stream"; + + /** + * Formats browsers render without executing anything the file carries. + * SVG is deliberately absent: it is an XML document that can carry script. + */ + private static final Set INLINE_TYPES = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList( + "image/jpeg", "image/jpg", "image/pjpeg", "image/png", + "image/x-png", "image/apng", "image/avif", "image/gif", + "image/bmp", "image/x-ms-bmp", "image/webp", "image/tiff", + "image/x-icon", "image/vnd.microsoft.icon", + "application/pdf", "text/plain"))); + + /** Families served inline whatever the subtype. */ + private static final String[] INLINE_PREFIXES = {"audio/", "video/"}; + + /** + * Types a browser may execute, or that can carry something it will. These + * are never adopted from a client's declaration. + */ + private static final Set ACTIVE_TYPES = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList( + "text/html", "application/xhtml+xml", "application/xhtml", + "image/svg+xml", "text/xml", "application/xml", + "text/javascript", "application/javascript", + "application/ecmascript", "text/ecmascript", + "text/vbscript", "application/x-shockwave-flash", + "text/xsl", "application/xslt+xml"))); + + /** + * @param fileName the uploaded file's name + * @param declaredType the type the client said it was, may be null + * @return the type to store: derived from the name where that is + * conclusive, otherwise the declared type if it is not one a + * browser would act on, otherwise the generic binary type + */ + public static String storedTypeFor(String fileName, String declaredType, + MimeTypeResolver resolver) { + String derived = typeFromName(fileName, resolver); + if (isConclusive(derived)) { + return derived; + } + + String declared = normalizeType(declaredType); + if (isConclusive(declared) && !isActive(declared)) { + return declared; + } + + return DEFAULT_TYPE; + } + + /** Resolves a file name through the servlet container's MIME mappings. */ + public static String typeFromName(String fileName, MimeTypeResolver resolver) { + return normalizeType(deriveFromName(fileName, resolver)); + } + + /** @return true when browsers render this type without executing it */ + public static boolean isInlineSafe(String contentType) { + String type = normalizeType(contentType); + if (type == null) { + return false; + } + if (INLINE_TYPES.contains(type)) { + return true; + } + for (String prefix : INLINE_PREFIXES) { + if (type.startsWith(prefix)) { + return true; + } + } + return false; + } + + /** @return true when a browser may execute this type, or script inside it */ + public static boolean isActive(String contentType) { + String type = normalizeType(contentType); + if (type == null) { + return false; + } + return ACTIVE_TYPES.contains(type) || type.endsWith("+xml"); + } + + /** + * Sets the type and the headers that govern how the response is treated. + * Anything outside the inline list is marked as an attachment. + */ + public static void applyResponseHeaders(HttpServletResponse response, + String contentType, String fileName) { + response.setHeader("X-Content-Type-Options", "nosniff"); + + String type = responseTypeFor(contentType); + + if (isInlineSafe(type)) { + response.setContentType(type); + return; + } + + // Served as bytes to be saved rather than a document to be rendered. + response.setContentType(DEFAULT_TYPE); + response.setHeader("Content-Disposition", + "attachment; filename=\"" + headerSafe(fileName) + "\""); + } + + /** @return the response type after applying the inline policy. */ + public static String responseTypeFor(String contentType) { + String type = normalizeType(contentType); + return type != null && isInlineSafe(type) ? type : DEFAULT_TYPE; + } + + private static String deriveFromName(String fileName, MimeTypeResolver resolver) { + if (fileName == null || fileName.trim().isEmpty() || resolver == null) { + return null; + } + String name = fileName.trim(); + int dot = name.lastIndexOf('.'); + if (dot > -1) { + name = name.substring(0, dot) + name.substring(dot).toLowerCase(Locale.ENGLISH); + } + return resolver.resolve(name); + } + + /** @return the bare type in lower case, without parameters such as charset */ + public static String normalizeType(String contentType) { + if (contentType == null) { + return null; + } + String type = contentType.trim(); + int semicolon = type.indexOf(';'); + if (semicolon > -1) { + type = type.substring(0, semicolon).trim(); + } + return type.isEmpty() ? null : type.toLowerCase(Locale.ENGLISH); + } + + private static boolean isConclusive(String type) { + return type != null && !DEFAULT_TYPE.equals(type); + } + + /** Resolves a name with the MIME mappings supplied by the active servlet container. */ + @FunctionalInterface + public interface MimeTypeResolver { + String resolve(String fileName); + } + + /** + * @return the name with the characters that would end the quoted string or + * start another header removed, since it is placed in one + */ + private static String headerSafe(String fileName) { + if (fileName == null || fileName.trim().isEmpty()) { + return "download"; + } + String safe = fileName.replaceAll("[\\r\\n\"\\\\]", ""); + return safe.trim().isEmpty() ? "download" : safe; + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java index e7e963482..b4b100fb8 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java @@ -44,9 +44,11 @@ import java.util.StringTokenizer; import java.util.TreeSet; import java.util.UUID; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.FileIOException; import org.apache.roller.weblogger.business.MediaFileManager; @@ -58,6 +60,7 @@ import org.apache.roller.weblogger.pojos.MediaFileDirectory; import org.apache.roller.weblogger.pojos.User; import org.apache.roller.weblogger.pojos.Weblog; +import org.apache.roller.weblogger.ui.core.RollerContext; import org.apache.roller.weblogger.util.RollerMessages; import org.apache.roller.weblogger.util.Utilities; @@ -69,6 +72,7 @@ public class MediaCollection { private Weblogger roller; private User user; + private HttpServletResponse response; private static final int MAX_ENTRIES = 20; private final String atomURL; @@ -77,8 +81,13 @@ public class MediaCollection { public MediaCollection(User user, String atomURL) { + this(user, atomURL, null); + } + + public MediaCollection(User user, String atomURL, HttpServletResponse response) { this.user = user; this.atomURL = atomURL; + this.response = response; this.roller = WebloggerFactory.getWeblogger(); } @@ -136,11 +145,18 @@ public Entry postMedia(AtomRequest areq, Entry entry) throws AtomException { mf.setWeblog(website); mf.setName(fileName); mf.setOriginalPath(justPath); - mf.setContentType(contentType); mf.setInputStream(fis); mf.setLength(tempFile.length()); RollerMessages errors = new RollerMessages(); + String declaredType = MediaTypePolicy.normalizeType(contentType); + if (!roller.getFileContentManager().canSave(website, fileName, + declaredType, tempFile.length(), errors)) { + throw new AtomException(errors.toString()); + } + mf.setContentType(MediaTypePolicy.storedTypeFor(fileName, + declaredType, + RollerContext.getServletContext()::getMimeType)); fileMgr.createMediaFile(website, mf, errors); if (errors.getErrorCount() > 0) { throw new AtomException(errors.toString()); @@ -220,11 +236,7 @@ public AtomMediaResource getMediaResource(AtomRequest areq) throws AtomException // Parse pathinfo to determine file path String filePath = filePathFromPathInfo(pathInfo); MediaFile mf = fmgr.getMediaFileByOriginalPath(website, filePath); - return new AtomMediaResource( - mf.getName(), - mf.getLength(), - new Date(mf.getLastModified()), - mf.getInputStream()); + return createMediaResource(mf, response); } catch (Exception e) { throw new AtomException( "Unexpected error during file upload", e); @@ -394,7 +406,16 @@ public void putMedia(AtomRequest areq) throws AtomException { // Attempt to load file, to ensure it exists MediaFile mf = fmgr.getMediaFileByPath(website, path); - mf.setContentType(contentType); + String replacementName = pathInfo[pathInfo.length - 1]; + String declaredType = MediaTypePolicy.normalizeType(contentType); + RollerMessages errors = new RollerMessages(); + if (!roller.getFileContentManager().canSave(website, + replacementName, declaredType, tempFile.length(), errors)) { + throw new FileIOException(errors.toString()); + } + mf.setContentType(MediaTypePolicy.storedTypeFor( + replacementName, declaredType, + RollerContext.getServletContext()::getMimeType)); mf.setInputStream(fis); mf.setLength(tempFile.length()); @@ -479,6 +500,21 @@ private String filePathFromPathInfo(String[] pathInfo) { } return path; } + + static AtomMediaResource createMediaResource(MediaFile mediaFile, + HttpServletResponse response) throws IOException { + AtomMediaResource resource = new AtomMediaResource( + mediaFile.getName(), mediaFile.getLength(), + new Date(mediaFile.getLastModified()), mediaFile.getInputStream()); + resource.setContentType(mediaFile.getContentType()); + if (response != null) { + MediaTypePolicy.applyResponseHeaders(response, + mediaFile.getContentType(), mediaFile.getName()); + resource.setContentType( + MediaTypePolicy.responseTypeFor(mediaFile.getContentType())); + } + return resource; + } private Entry createAtomResourceEntry(Weblog website, MediaFile file) { String filePath = file.getPath().endsWith("/") diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java index 55e1c576e..070741cfd 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java @@ -96,6 +96,7 @@ public class RollerAtomHandler implements AtomHandler { protected User user = null; protected int maxEntries = 20; protected String atomURL = null; + private final HttpServletResponse response; protected static final boolean THROTTLE; @@ -115,6 +116,7 @@ public class RollerAtomHandler implements AtomHandler { * then user's name, otherwise it will return null. */ public RollerAtomHandler(HttpServletRequest request, HttpServletResponse response) { + this.response = response; roller = WebloggerFactory.getWeblogger(); String userName; @@ -253,7 +255,7 @@ public Entry getEntry(AtomRequest areq) throws AtomException { */ @Override public AtomMediaResource getMediaResource(AtomRequest areq) throws AtomException { - MediaCollection mcol = new MediaCollection(user, atomURL); + MediaCollection mcol = new MediaCollection(user, atomURL, response); return mcol.getMediaResource(areq); } diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java b/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java index dc8ce3f69..e19ec5213 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java @@ -29,6 +29,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.util.RollerConstants; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.URLStrategy; import org.apache.roller.weblogger.business.WeblogEntryManager; @@ -43,6 +44,7 @@ import org.apache.roller.weblogger.pojos.WeblogEntry; import org.apache.roller.weblogger.pojos.WeblogEntry.PubStatus; import org.apache.roller.weblogger.pojos.WeblogEntrySearchCriteria; +import org.apache.roller.weblogger.ui.core.RollerContext; import org.apache.roller.weblogger.util.RollerMessages; import org.apache.roller.weblogger.util.Utilities; import org.apache.xmlrpc.XmlRpcException; @@ -381,12 +383,18 @@ public Object newMediaObject(String blogid, String userid, String password, mf.setDirectory(root); mf.setWeblog(website); mf.setName(name); - mf.setContentType(type); mf.setInputStream(new ByteArrayInputStream(bits)); mf.setLength(bits.length); String fileLink = mf.getPermalink(); RollerMessages errors = new RollerMessages(); + String declaredType = MediaTypePolicy.normalizeType(type); + if (!roller.getFileContentManager().canSave(website, name, + declaredType, bits.length, errors)) { + throw new Exception(errors.toString()); + } + mf.setContentType(MediaTypePolicy.storedTypeFor(name, declaredType, + RollerContext.getServletContext()::getMimeType)); fmgr.createMediaFile(website, mf, errors); if (errors.getErrorCount() > 0) { diff --git a/app/src/test/java/org/apache/roller/weblogger/business/themes/WeblogCustomThemeTest.java b/app/src/test/java/org/apache/roller/weblogger/business/themes/WeblogCustomThemeTest.java new file mode 100644 index 000000000..0c4851117 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/business/themes/WeblogCustomThemeTest.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0. + */ +package org.apache.roller.weblogger.business.themes; + +import java.io.ByteArrayInputStream; + +import org.apache.roller.weblogger.business.MediaFileManager; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.pojos.MediaFile; +import org.apache.roller.weblogger.pojos.ThemeResource; +import org.apache.roller.weblogger.pojos.Weblog; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +class WeblogCustomThemeTest { + + @Test + void mediaFileBackedResourcesAreReturnedAsThemeResources() throws Exception { + Weblog weblog = mock(Weblog.class); + Weblogger roller = mock(Weblogger.class); + MediaFileManager manager = mock(MediaFileManager.class); + MediaFile mediaFile = mock(MediaFile.class); + ByteArrayInputStream content = new ByteArrayInputStream(new byte[]{1, 2, 3}); + + when(roller.getMediaFileManager()).thenReturn(manager); + when(manager.getMediaFileByOriginalPath(weblog, "css/site.css")) + .thenReturn(mediaFile); + when(mediaFile.getName()).thenReturn("site.css"); + when(mediaFile.getOriginalPath()).thenReturn("css/site.css"); + when(mediaFile.getLength()).thenReturn(3L); + when(mediaFile.getLastModified()).thenReturn(7L); + when(mediaFile.getInputStream()).thenReturn(content); + + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + factory.when(WebloggerFactory::getWeblogger).thenReturn(roller); + + ThemeResource resource = new WeblogCustomTheme(weblog) + .getResource("css/site.css"); + + assertNotNull(resource); + assertEquals("site.css", resource.getName()); + assertEquals("css/site.css", resource.getPath()); + assertEquals(3L, resource.getLength()); + assertEquals(7L, resource.getLastModified()); + assertSame(content, resource.getInputStream()); + } + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java b/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java new file mode 100644 index 000000000..2cd9a850d --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java @@ -0,0 +1,277 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. For additional + * information regarding copyright in this work, please see the NOTICE + * file in the top level directory of this distribution. + */ +package org.apache.roller.weblogger.util; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import javax.servlet.http.HttpServletResponse; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * What an uploaded file is stored as, and how it comes back. + * + *

The type a client puts on an upload states what the sender meant to send, + * not what the bytes are, and the browser acts on whatever Roller repeats back. + * These cases fix both ends of that: which type is kept, and which types are + * allowed to render as a document rather than download. + */ +public class MediaTypePolicyTest { + + // ----------------------------------------------------------------- // + // Stored type + // ----------------------------------------------------------------- // + + /** The file name decides, so a declaration cannot contradict it. */ + @Test + public void theFileNameDecidesTheStoredType() { + assertEquals("image/jpeg", + MediaTypePolicy.storedTypeFor("holiday.jpg", "text/html", + MediaTypePolicyTest::mimeTypeFor), + "a declared type overrode the file name"); + assertEquals("image/png", + MediaTypePolicy.storedTypeFor("diagram.png", "image/svg+xml", + MediaTypePolicyTest::mimeTypeFor)); + assertEquals("image/gif", + MediaTypePolicy.storedTypeFor("loop.GIF", "application/xhtml+xml", + MediaTypePolicyTest::mimeTypeFor), + "the extension must be matched regardless of case"); + } + + /** Where the name says nothing, an executable declaration is still refused. */ + @Test + public void anExecutableDeclarationIsNeverAdopted() { + for (String active : new String[]{ + "text/html", "text/html; charset=utf-8", "application/xhtml+xml", + "image/svg+xml", "application/xml", "text/javascript", + "application/javascript", "text/xsl", "something/custom+xml"}) { + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.storedTypeFor("payload.unknownext", active, + MediaTypePolicyTest::mimeTypeFor), + "adopted an executable declared type: " + active); + } + } + + /** A harmless declaration is still useful where the name is opaque. */ + @Test + public void aHarmlessDeclarationIsUsedWhenTheNameIsOpaque() { + assertEquals("application/zip", + MediaTypePolicy.storedTypeFor("bundle.unknownext", "application/zip", + MediaTypePolicyTest::mimeTypeFor)); + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.storedTypeFor("bundle.unknownext", null, + MediaTypePolicyTest::mimeTypeFor)); + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.storedTypeFor(null, null, + MediaTypePolicyTest::mimeTypeFor)); + } + + // ----------------------------------------------------------------- // + // Inline policy + // ----------------------------------------------------------------- // + + @Test + public void passiveFormatsRenderInline() { + for (String inline : new String[]{ + "image/jpeg", "image/jpg", "image/png", "image/x-png", + "image/apng", "image/avif", "image/gif", "image/webp", + "application/pdf", "text/plain", "audio/mpeg", "video/mp4", + "image/png; charset=binary"}) { + assertTrue(MediaTypePolicy.isInlineSafe(inline), + "expected to render inline: " + inline); + } + } + + @Test + public void formatsThatCanCarryScriptDoNot() { + for (String blocked : new String[]{ + "image/svg+xml", "text/html", "application/xhtml+xml", + "text/xml", "application/javascript", + "application/zip", null, ""}) { + assertFalse(MediaTypePolicy.isInlineSafe(blocked), + "expected not to render inline: " + blocked); + } + } + + // ----------------------------------------------------------------- // + // Response headers + // ----------------------------------------------------------------- // + + @Test + public void everyResponseDeclaresNosniff() { + for (String type : new String[]{"image/png", "text/html", null}) { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, type, "f.bin"); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + } + } + + @Test + public void anInlineTypeKeepsItsTypeAndIsNotAnAttachment() { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, "image/png", "diagram.png"); + verify(response).setContentType("image/png"); + verify(response, never()).setHeader(eq("Content-Disposition"), anyString()); + } + + @Test + public void anythingElseIsSentAsAnAttachment() { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, "text/html", "page.html"); + verify(response).setContentType(MediaTypePolicy.DEFAULT_TYPE); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"page.html\""); + } + + /** The name is placed inside a header, so it cannot be allowed to leave it. */ + @Test + public void theAttachmentNameCannotBreakOutOfTheHeader() { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, "text/html", + "evil\r\nSet-Cookie: a=b\".html"); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"evilSet-Cookie: a=b.html\""); + } + + @Test + public void servletMappingsAreUsedCaseInsensitively() { + assertEquals("application/pdf", MediaTypePolicy.typeFromName( + "report.PDF", MediaTypePolicyTest::mimeTypeFor)); + assertEquals("video/mp4", MediaTypePolicy.typeFromName( + "clip.MP4", MediaTypePolicyTest::mimeTypeFor)); + assertEquals(null, MediaTypePolicy.typeFromName( + "payload.unmapped", MediaTypePolicyTest::mimeTypeFor)); + } + + @Test + public void unknownAndActiveTypesUseTheDownloadType() { + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.responseTypeFor(null)); + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.responseTypeFor("image/svg+xml")); + assertEquals("text/plain", + MediaTypePolicy.responseTypeFor("text/plain; charset=utf-8")); + } + + private static String mimeTypeFor(String fileName) { + if (fileName.endsWith(".jpg")) return "image/jpeg"; + if (fileName.endsWith(".png")) return "image/png"; + if (fileName.endsWith(".gif")) return "image/gif"; + if (fileName.endsWith(".pdf")) return "application/pdf"; + if (fileName.endsWith(".mp4")) return "video/mp4"; + return null; + } + + // ----------------------------------------------------------------- // + // The callers actually use it + // ----------------------------------------------------------------- // + + private String source(String relativePath) throws Exception { + Path path = Paths.get("src", "main", "java"); + for (String segment : relativePath.split("/")) { + path = path.resolve(segment); + } + assertTrue(Files.isReadable(path), + "cannot read " + path.toAbsolutePath() + " (run from the app module)"); + return new String(Files.readAllBytes(path), StandardCharsets.UTF_8); + } + + /** + * The serving path must not set a type of its own, or the headers above + * are decided somewhere this test cannot see. + */ + @Test + public void theServingPathGoesThroughThePolicy() throws Exception { + String servlet = source("org/apache/roller/weblogger/ui/rendering/" + + "servlets/MediaResourceServlet.java"); + assertTrue(servlet.contains("MediaTypePolicy.applyResponseHeaders"), + "MediaResourceServlet must apply the policy to its response"); + assertFalse(servlet.contains("response.setContentType("), + "MediaResourceServlet must not set a content type directly"); + } + + /** + * Uploaded media is also reachable through the two resource servlets, which + * serve theme resources from the same method. Only the uploaded-media + * branch takes the media policy — applying it to theme resources would send + * every stylesheet as a download — but both branches must refuse sniffing. + */ + @Test + public void theResourceServletsCoverTheirUploadedMediaBranch() throws Exception { + for (String name : new String[]{"ResourceServlet", "PreviewResourceServlet"}) { + String servlet = source("org/apache/roller/weblogger/ui/rendering/" + + "servlets/" + name + ".java"); + assertTrue(servlet.contains("MediaTypePolicy.applyResponseHeaders"), + name + " must apply the media policy to uploaded media"); + assertTrue(servlet.contains("fromUploadedMedia"), + name + " must distinguish uploaded media from theme resources"); + assertTrue(servlet.contains("X-Content-Type-Options"), + name + " must refuse sniffing on the theme branch too"); + } + } + + /** Each upload path must derive the stored type rather than take it. */ + @Test + public void everyUploadPathGoesThroughThePolicy() throws Exception { + String[][] callers = { + {"org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java", + "this.uploadedFilesContentType[i]"}, + {"org/apache/roller/weblogger/webservices/atomprotocol/" + + "MediaCollection.java", "setContentType(contentType)"}, + {"org/apache/roller/weblogger/webservices/xmlrpc/" + + "MetaWeblogAPIHandler.java", "setContentType(type)"}, + // Replacing an existing file's body is an upload too. + {"org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java", + "this.uploadedFileContentType"}, + }; + for (String[] caller : callers) { + String src = source(caller[0]); + assertTrue(src.contains("MediaTypePolicy.storedTypeFor"), + caller[0] + " must derive the stored type through the policy"); + assertTrue(src.contains("getFileContentManager().canSave"), + caller[0] + " must validate the declared type before deriving it"); + assertFalse(src.contains("setContentType(" + caller[1] + ")"), + caller[0] + " still stores the client's declared type directly"); + } + } + + @Test + public void replacementRoutesUseTheReplacementName() throws Exception { + String editor = source("org/apache/roller/weblogger/ui/struts2/editor/" + + "MediaFileEdit.java"); + assertTrue(editor.contains("storedTypeFor(\n this.uploadedFileName")); + + String atom = source("org/apache/roller/weblogger/webservices/atomprotocol/" + + "MediaCollection.java"); + assertTrue(atom.contains("storedTypeFor(\n replacementName")); + assertTrue(atom.contains("createMediaResource(mf, response)"), + "Atom media reads must apply the response policy"); + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java new file mode 100644 index 000000000..c374d2adc --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0. + */ +package org.apache.roller.weblogger.webservices.atomprotocol; + +import java.io.ByteArrayInputStream; + +import javax.servlet.http.HttpServletResponse; + +import com.rometools.propono.atom.server.AtomMediaResource; +import org.apache.roller.weblogger.pojos.MediaFile; +import org.apache.roller.weblogger.util.MediaTypePolicy; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class MediaCollectionTest { + + @Test + void activeStoredTypeIsReturnedAsADownload() throws Exception { + MediaFile mediaFile = mediaFile("page.html", "text/html"); + HttpServletResponse response = mock(HttpServletResponse.class); + + AtomMediaResource resource = MediaCollection.createMediaResource(mediaFile, response); + + assertEquals(MediaTypePolicy.DEFAULT_TYPE, resource.getContentType()); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"page.html\""); + verify(response).setContentType(MediaTypePolicy.DEFAULT_TYPE); + } + + @Test + void passiveStoredTypeRemainsInline() throws Exception { + MediaFile mediaFile = mediaFile("notes.txt", "text/plain"); + HttpServletResponse response = mock(HttpServletResponse.class); + + AtomMediaResource resource = MediaCollection.createMediaResource(mediaFile, response); + + assertEquals("text/plain", resource.getContentType()); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + verify(response).setContentType("text/plain"); + } + + private MediaFile mediaFile(String name, String type) { + MediaFile mediaFile = mock(MediaFile.class); + when(mediaFile.getName()).thenReturn(name); + when(mediaFile.getContentType()).thenReturn(type); + when(mediaFile.getLength()).thenReturn(3L); + when(mediaFile.getLastModified()).thenReturn(7L); + when(mediaFile.getInputStream()) + .thenReturn(new ByteArrayInputStream(new byte[]{1, 2, 3})); + return mediaFile; + } +}