Bug #14784

Fix the vulnerability by setting a restrictive content security policy
of the HTTP response when serving a file in Silverpeas.
Now, when serving a file, it is done explicitly for download and
not anymore for inlining its content.

Author: mmoqui

core-web/src/main/java/org/silverpeas/core/web/http/FileResponse.java

@@ -45,7 +45,6 @@
 import java.time.Instant;
 import java.util.Base64;
 import java.util.Date;
-import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -59,7 +58,6 @@
  */
 public abstract class FileResponse {
 
-  public static final String DOWNLOAD_CONTEXT_PARAM = "downloadContext";
   private static final int MAX_PATH_LENGTH_IN_LOGS = 100;
   private static final int BUFFER_LENGTH = 1024 * 16;
   private static final Pattern RANGE_PATTERN = Pattern.compile("bytes=(?<start>\\d*)-(?<end>\\d*)");
@@ -82,6 +80,8 @@ public abstract class FileResponse {
   FileResponse(final HttpServletRequest request, final HttpServletResponse response) {
     this.request = request;
     this.response = response;
+    response.setHeader("Content-Security-Policy",
+        "default-src 'none'; img-src 'self'; style-src 'none'; script-src 'none'");
   }
 
   /**
@@ -128,20 +128,6 @@ public static ServletFileResponse fromServlet(final HttpServletRequest request,
     return new ServletFileResponse(request, response);
   }
 
-  /**
-   * Indicates a download context if any set into request.
-   * @return true if download context, false otherwise.
-   */
-  boolean isDownloadContext() {
-    final String parameter = Optional.ofNullable(request.getParameter(DOWNLOAD_CONTEXT_PARAM))
-        .filter(StringUtil::isDefined)
-        .orElseGet(() -> {
-          final Object attribute = request.getAttribute(DOWNLOAD_CONTEXT_PARAM);
-          return attribute == null ? "false" : attribute.toString();
-        });
-    return StringUtil.getBooleanValue(parameter);
-  }
-
   /**
    * Gets mime type.
    * @param absoluteFilePath the absolute file path.
@@ -251,7 +237,7 @@ void partialOutputStream(final Path path, final ContentRangeData partialData,
       ByteBuffer buffer = ByteBuffer.allocate(BUFFER_LENGTH);
       while ((bytesRead = input.read(buffer)) != -1 && bytesLeft > 0) {
         buffer.clear();
-        output.write(buffer.array(), 0, bytesLeft < bytesRead ? bytesLeft : bytesRead);
+        output.write(buffer.array(), 0, Math.min(bytesLeft, bytesRead));
         bytesLeft -= bytesRead;
       }
       SilverLogger.getLogger(this).debug("{0} - all part content bytes sent", StringUtil
@@ -298,7 +284,7 @@ String getFileIdentifier(final Path path) throws IOException {
     }
     final File file = path.toFile();
     final String sb =
-            String.valueOf(FileUtils.checksumCRC32(file)) +
+        FileUtils.checksumCRC32(file) +
             "|" + file.getName() +
             "|" + file.length() +
             "|" + file.lastModified();
@@ -343,12 +329,12 @@ ContentRangeData getContentRangeData(final Matcher partialHeaders, final int ful
     response.setBufferSize(BUFFER_LENGTH);
 
     String startGroup = partialHeaders.group("start");
-    int start = startGroup.isEmpty() ? 0 : Integer.valueOf(startGroup);
-    start = start < 0 ? 0 : start;
+    int start = startGroup.isEmpty() ? 0 : Integer.parseInt(startGroup);
+    start = Math.max(start, 0);
 
     String endGroup = partialHeaders.group("end");
-    int end = endGroup.isEmpty() ? endOfFullContent : Integer.valueOf(endGroup);
-    end = end > endOfFullContent ? endOfFullContent : end;
+    int end = endGroup.isEmpty() ? endOfFullContent : Integer.parseInt(endGroup);
+    end = Math.min(end, endOfFullContent);
 
     int partContentLength = end - start + 1;
     return new ContentRangeData(start, end, partContentLength,

core-web/src/main/java/org/silverpeas/core/web/http/PreparedDownload.java

@@ -102,7 +102,7 @@ public static Optional<PreparedDownload> getPreparedDownloadToPerform(
    * Gets from the given request the {@link PreparedDownload} instance registered into the
    * session if any.
    * @param request the request.
-   * @throw IllegalArgumentException if parameter id is not defined into request or if it does
+   * @throws IllegalArgumentException if parameter id is not defined into request or if it does
    * not exists {@link PreparedDownload} instance referenced by the identifier.
    * @return a {@link PreparedDownload} instance.
    */
@@ -170,7 +170,7 @@ public void sendTo(final HttpServletRequest request, final HttpServletResponse r
                   .forceCharacterEncoding(characterEncoding)
                   .forceFileName(fileName)
                   .noCache()
-                  .sendPath(Paths.get(file.toURI()), true);
+                  .sendPath(Paths.get(file.toURI()));
     } finally {
       deleteQuietly(file);
     }

core-web/src/main/java/org/silverpeas/core/web/http/RestFileResponse.java

@@ -55,53 +55,35 @@ public class RestFileResponse extends FileResponse {
   }
 
   /**
-   * Centralization of getting of silverpeas file content.
-   * <p>
-   * A download context flag is verified from parameters and attributes of the request.
-   * </p>
+   * Centralization of getting the silverpeas file content. By default, the file will be for
+   * download and not for viewing its content directly in the client (as the file content can
+   * contain corrupting code).
    * @param file the silverpeas file to send.
    * @return the response builder.
    */
   public Response.ResponseBuilder silverpeasFile(final SilverpeasFile file) {
-    return silverpeasFile(file, isDownloadContext());
-  }
-
-  /**
-   * Centralization of getting of silverpeas file content.
-   * <p>
-   * Using directly this method means that the request downloadContext flag is ignored.
-   * </p>
-   * @param file the silverpeas file to send.
-   * @param downloadContext indicating a download context in order to specify rightly response
-   * headers.
-   * @return the response builder.
-   */
-  public Response.ResponseBuilder silverpeasFile(final SilverpeasFile file,
-      final boolean downloadContext) {
     if (isNotDefined(forcedMimeType)) {
       forceMimeType(file.getMimeType());
     }
-    return path(Paths.get(file.toURI()), downloadContext);
+    return path(Paths.get(file.toURI()));
   }
 
   /**
    * Centralization of getting of a file content.
    * @param path the file to send.
-   * @param downloadContext indicating a download context in order to specify rightly response
    * headers.
    * @return the response.
    */
-  private Response.ResponseBuilder path(final Path path, final boolean downloadContext) {
+  private Response.ResponseBuilder path(final Path path) {
     try {
-      final Path absoluteFilePath = path.toAbsolutePath();
-      final String fileName = getFileName(absoluteFilePath);
-      final String fileMimeType = getMimeType(absoluteFilePath);
-
-      final int fullContentLength = (int) Files.size(absoluteFilePath);
-      final Matcher partialMatcher = getPartialMatcher();
-      final boolean isPartialRequest = partialMatcher.matches();
-
-      final Response.ResponseBuilder responseBuilder;
+      var absoluteFilePath = path.toAbsolutePath();
+      var fileName = getFileName(absoluteFilePath);
+      var fileMimeType = getMimeType(absoluteFilePath);
+      var fullContentLength = (int) Files.size(absoluteFilePath);
+      var partialMatcher = getPartialMatcher();
+      var isPartialRequest = partialMatcher.matches();
+
+      Response.ResponseBuilder responseBuilder;
       if (isPartialRequest) {
         // Handling here a partial response (pseudo streaming)
         responseBuilder =
@@ -111,9 +93,7 @@ private Response.ResponseBuilder path(final Path path, final boolean downloadCon
         responseBuilder = getFullResponseBuilder(absoluteFilePath, fullContentLength);
       }
 
-      final String filename = downloadContext
-          ? encodeAttachmentFilenameAsUtf8(fileName)
-          : encodeInlineFilenameAsUtf8(fileName);
+      var filename = encodeAttachmentFilenameAsUtf8(fileName);
       return responseBuilder.type(fileMimeType).header("Content-Disposition", filename);
     } catch (final WebApplicationException ex) {
       throw ex;

core-web/src/main/java/org/silverpeas/core/web/http/ServletFileResponse.java

@@ -55,53 +55,34 @@ public class ServletFileResponse extends FileResponse {
   }
 
   /**
-   * Centralization of getting of silverpeas file content.
-   * <p>
-   * A download context flag is verified from parameters and attributes of the request.
-   * </p>
+   * Centralization of getting of silverpeas file content. By default, the file will be for
+   * download and not for viewing its content directly in the client (as the file content can
+   * contain corrupting code).
    * @param file the silverpeas file to send.
    */
   public void sendSilverpeasFile(final SilverpeasFile file) {
-    sendSilverpeasFile(file, isDownloadContext());
-  }
-
-  /**
-   * Centralization of getting of silverpeas file content.
-   * <p>
-   * Using directly this method means that the request downloadContext flag is ignored.
-   * </p>
-   * @param file the silverpeas file to send.
-   * @param downloadContext indicating a download context in order to specify rightly response
-   * headers.
-   */
-  public void sendSilverpeasFile(final SilverpeasFile file,
-      final boolean downloadContext) {
     if (isNotDefined(forcedMimeType)) {
       forceMimeType(file.getMimeType());
     }
-    sendPath(Paths.get(file.toURI()), downloadContext);
+    sendPath(Paths.get(file.toURI()));
   }
 
   /**
    * Centralization of getting of a file content.
    * @param path the file to send.
-   * @param downloadContext indicating a download context in order to specify rightly response
    * headers.
    */
-  void sendPath(final Path path, final boolean downloadContext) {
+  void sendPath(final Path path) {
     try {
-      final Path absoluteFilePath = path.toAbsolutePath();
-      final String fileName = getFileName(absoluteFilePath);
-      final String fileMimeType = getMimeType(absoluteFilePath);
-
-      final int fullContentLength = (int) Files.size(absoluteFilePath);
-      final Matcher partialMatcher = getPartialMatcher();
-      final boolean isPartialRequest = partialMatcher.matches();
+      Path absoluteFilePath = path.toAbsolutePath();
+      String fileName = getFileName(absoluteFilePath);
+      String fileMimeType = getMimeType(absoluteFilePath);
+      int fullContentLength = (int) Files.size(absoluteFilePath);
+      Matcher partialMatcher = getPartialMatcher();
+      boolean isPartialRequest = partialMatcher.matches();
 
       response.setContentType(fileMimeType);
-      final String filename = downloadContext
-          ? encodeAttachmentFilenameAsUtf8(fileName)
-          : encodeInlineFilenameAsUtf8(fileName);
+      final String filename = encodeAttachmentFilenameAsUtf8(fileName);
       response.setHeader("Content-Disposition", filename);
       if (isPartialRequest) {
         // Handling here a partial response (pseudo streaming)