feat: add more validation to file upload (#18112)

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/FileResourceBlocklist.java

@@ -34,7 +34,7 @@
  * @author Lars Helge Overland
  */
 public class FileResourceBlocklist {
-  private static final ImmutableSet<String> CONTENT_TYPES =
+  private static final ImmutableSet<String> BLOCKED_CONTENT_TYPES =
       ImmutableSet.of(
           // Web
           "text/html",
@@ -54,7 +54,7 @@ public class FileResourceBlocklist {
           "application/x-sh",
           "application/x-csh");
 
-  private static final ImmutableSet<String> FILE_EXTENSIONS =
+  private static final ImmutableSet<String> BLOCKED_FILE_EXTENSIONS =
       ImmutableSet.of(
           // Web
           "html",
@@ -93,11 +93,11 @@ public static boolean isValid(FileResource fileResource) {
       return false;
     }
 
-    if (CONTENT_TYPES.contains(fileResource.getContentType().toLowerCase())) {
+    if (BLOCKED_CONTENT_TYPES.contains(fileResource.getContentType().toLowerCase())) {
       return false;
     }
 
-    if (FILE_EXTENSIONS.contains(
+    if (BLOCKED_FILE_EXTENSIONS.contains(
         FilenameUtils.getExtension(fileResource.getName().toLowerCase()))) {
       return false;
     }

dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/FileResourceControllerTest.java

@@ -29,32 +29,38 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
 import org.hisp.dhis.feedback.ErrorCode;
 import org.hisp.dhis.jsontree.JsonObject;
 import org.hisp.dhis.web.HttpStatus;
 import org.hisp.dhis.webapi.DhisControllerConvenienceTest;
 import org.hisp.dhis.webapi.json.domain.JsonWebMessage;
 import org.junit.jupiter.api.Test;
+import org.springframework.core.io.ClassPathResource;
 import org.springframework.mock.web.MockMultipartFile;
 
 class FileResourceControllerTest extends DhisControllerConvenienceTest {
 
   @Test
-  void testSaveOrgUnitImage() {
+  void testSaveOrgUnitImage() throws IOException {
+    File file = new ClassPathResource("file/dhis2.png").getFile();
     MockMultipartFile image =
         new MockMultipartFile(
-            "file", "OU_profile_image.png", "image/png", "<<png data>>".getBytes());
+            "file", "OU_profile_image.png", "image/png", Files.readAllBytes(file.toPath()));
     HttpResponse response = POST_MULTIPART("/fileResources?domain=ORG_UNIT", image);
     JsonObject savedObject =
         response.content(HttpStatus.ACCEPTED).getObject("response").getObject("fileResource");
     assertEquals("OU_profile_image.png", savedObject.getString("name").string());
   }
 
   @Test
-  void testSaveOrgUnitImageWithUid() {
+  void testSaveOrgUnitImageWithUid() throws IOException {
+    File file = new ClassPathResource("file/dhis2.png").getFile();
     MockMultipartFile image =
         new MockMultipartFile(
-            "file", "OU_profile_image.png", "image/png", "<<png data>>".getBytes());
+            "file", "OU_profile_image.png", "image/png", Files.readAllBytes(file.toPath()));
     HttpResponse response = POST_MULTIPART("/fileResources?domain=ORG_UNIT&uid=0123456789a", image);
     JsonObject savedObject =
         response.content(HttpStatus.ACCEPTED).getObject("response").getObject("fileResource");
@@ -63,10 +69,11 @@ void testSaveOrgUnitImageWithUid() {
   }
 
   @Test
-  void testSaveOrgUnitImageWithUid_Update() {
+  void testSaveOrgUnitImageWithUid_Update() throws IOException {
+    File file = new ClassPathResource("file/dhis2.png").getFile();
     MockMultipartFile image =
         new MockMultipartFile(
-            "file", "OU_profile_image.png", "image/png", "<<png data>>".getBytes());
+            "file", "OU_profile_image.png", "image/png", Files.readAllBytes(file.toPath()));
     HttpResponse response = POST_MULTIPART("/fileResources?domain=ORG_UNIT&uid=0123456789x", image);
     JsonObject savedObject =
         response.content(HttpStatus.ACCEPTED).getObject("response").getObject("fileResource");
@@ -76,7 +83,7 @@ void testSaveOrgUnitImageWithUid_Update() {
     // now update the resource with a different image but the same UID
     MockMultipartFile image2 =
         new MockMultipartFile(
-            "file", "OU_profile_image2.png", "image/png", "<<png data>>".getBytes());
+            "file", "OU_profile_image2.png", "image/png", Files.readAllBytes(file.toPath()));
 
     JsonWebMessage message =
         assertWebMessage(

dhis-2/dhis-web-api/pom.xml

@@ -326,6 +326,12 @@
       <scope>provided</scope>
     </dependency>
 
+    <!-- Image Processing -->
+    <dependency>
+      <groupId>org.imgscalr</groupId>
+      <artifactId>imgscalr-lib</artifactId>
+    </dependency>
+
     <!-- Test -->
     <dependency>
       <groupId>org.hamcrest</groupId>

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/FileResourceController.java

@@ -30,6 +30,7 @@
 import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.error;
 import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.notFound;
 import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.unauthorized;
+import static org.hisp.dhis.webapi.utils.FileResourceUtils.resizeAvatarToDefaultSize;
 
 import com.google.common.base.MoreObjects;
 import java.io.IOException;
@@ -119,10 +120,12 @@ public void getFileResourceData(
     }
 
     response.setContentType(fileResource.getContentType());
+
     response.setHeader(
         HttpHeaders.CONTENT_LENGTH,
         String.valueOf(fileResourceService.getFileResourceContentLength(fileResource)));
     response.setHeader(HttpHeaders.CONTENT_DISPOSITION, "filename=" + fileResource.getName());
+
     HeaderUtils.setSecurityHeaders(
         response, dhisConfig.getProperty(ConfigurationKey.CSP_HEADER_VALUE));
 
@@ -143,8 +146,23 @@ public WebMessage saveFileResource(
       @RequestParam MultipartFile file,
       @RequestParam(defaultValue = "DATA_VALUE") FileResourceDomain domain,
       @RequestParam(required = false) String uid)
-      throws WebMessageException, IOException {
-    FileResource fileResource = fileResourceUtils.saveFileResource(uid, file, domain);
+      throws IOException, WebMessageException {
+    FileResource fileResource;
+
+    if (domain.equals(FileResourceDomain.USER_AVATAR)) {
+      fileResourceUtils.validateUserAvatar(file);
+      fileResource =
+          fileResourceUtils.saveFileResource(uid, resizeAvatarToDefaultSize(file), domain);
+
+    } else if (domain.equals(FileResourceDomain.ORG_UNIT)) {
+      fileResourceUtils.validateOrgUnitImage(file);
+      fileResource =
+          fileResourceUtils.saveFileResource(
+              uid, fileResourceUtils.resizeOrgToDefaultSize(file), domain);
+
+    } else {
+      fileResource = fileResourceUtils.saveFileResource(uid, file, domain);
+    }
 
     WebMessage webMessage = new WebMessage(Status.OK, HttpStatus.ACCEPTED);
     webMessage.setResponse(new FileResourceWebMessageResponse(fileResource));

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/utils/FileResourceUtils.java

@@ -27,43 +27,95 @@
  */
 package org.hisp.dhis.webapi.utils;
 
+import static org.apache.commons.io.FilenameUtils.getExtension;
 import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.conflict;
 import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.error;
 import static org.hisp.dhis.external.conf.ConfigurationKey.CSP_HEADER_VALUE;
+import static org.imgscalr.Scalr.resize;
 
 import com.google.common.hash.Hashing;
 import com.google.common.io.ByteSource;
+import java.awt.image.BufferedImage;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 import java.nio.file.Files;
+import java.util.List;
+import java.util.Objects;
+import javax.imageio.ImageIO;
 import javax.servlet.http.HttpServletResponse;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.fileupload.FileItem;
+import org.apache.commons.fileupload.disk.DiskFileItemFactory;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.input.NullInputStream;
 import org.apache.commons.lang3.StringUtils;
+import org.hisp.dhis.common.IllegalQueryException;
 import org.hisp.dhis.dxf2.webmessage.WebMessageException;
 import org.hisp.dhis.external.conf.DhisConfigurationProvider;
 import org.hisp.dhis.feedback.ErrorCode;
 import org.hisp.dhis.fileresource.FileResource;
 import org.hisp.dhis.fileresource.FileResourceDomain;
 import org.hisp.dhis.fileresource.FileResourceService;
 import org.hisp.dhis.fileresource.ImageFileDimension;
+import org.imgscalr.Scalr.Mode;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpHeaders;
 import org.springframework.stereotype.Component;
 import org.springframework.util.InvalidMimeTypeException;
 import org.springframework.util.MimeTypeUtils;
 import org.springframework.web.multipart.MultipartFile;
+import org.springframework.web.multipart.commons.CommonsMultipartFile;
 
 /**
  * @author Lars Helge Overland
  */
 @Component
 @Slf4j
 public class FileResourceUtils {
+
   @Autowired private FileResourceService fileResourceService;
 
+  private static final List<String> CUSTOM_ICON_VALID_ICON_EXTENSIONS = List.of("png");
+
+  private static final long CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES = 25_000_000;
+
+  private static final int CUSTOM_ICON_TARGET_HEIGHT = 48;
+  private static final int CUSTOM_ICON_TARGET_WIDTH = 48;
+
+  private static final int AVATAR_TARGET_HEIGHT = 200;
+  private static final int AVATAR_TARGET_WIDTH = 200;
+
+  private static final int ORGUNIT_TARGET_HEIGHT = 800;
+  private static final int ORGUNIT_TARGET_WIDTH = 800;
+
+  private static final long MAX_AVATAR_IMAGE_SIZE_IN_BYTES = 2_000_000;
+  private static final long MAX_ORGUNIT_IMAGE_SIZE_IN_BYTES = 8_000_000;
+
+  private static final List<String> ALLOWED_IMAGE_FILE_EXTENSIONS =
+      List.of("jpg", "jpeg", "png", "gif");
+  private static final List<String> ALLOWED_IMAGE_MIME_TYPES =
+      List.of("image/jpeg", "image/png", "image/gif");
+
+  private static class MultipartFileByteSource extends ByteSource {
+    private MultipartFile file;
+
+    public MultipartFileByteSource(MultipartFile file) {
+      this.file = file;
+    }
+
+    @Override
+    public InputStream openStream() {
+      try {
+        return file.getInputStream();
+      } catch (IOException ioe) {
+        return new NullInputStream(0);
+      }
+    }
+  }
+
   /**
    * Transfers the given multipart file content to a local temporary file.
    *
@@ -197,20 +249,121 @@ public FileResource saveFileResource(String uid, MultipartFile file, FileResourc
   // Inner classes
   // -------------------------------------------------------------------------
 
-  private class MultipartFileByteSource extends ByteSource {
-    private MultipartFile file;
+  //  private class MultipartFileByteSource extends ByteSource {
+  //    private MultipartFile file;
+  //
+  //    public MultipartFileByteSource(MultipartFile file) {
+  //      this.file = file;
+  //    }
+  //
+  //    @Override
+  //    public InputStream openStream() throws IOException {
+  //      try {
+  //        return file.getInputStream();
+  //      } catch (IOException ioe) {
+  //        return new NullInputStream(0);
+  //      }
+  //    }
+  //    }
 
-    public MultipartFileByteSource(MultipartFile file) {
-      this.file = file;
+  public void validateOrgUnitImage(MultipartFile file) {
+    validateContentType(file.getContentType(), ALLOWED_IMAGE_MIME_TYPES);
+    validateFileExtension(file.getOriginalFilename(), ALLOWED_IMAGE_FILE_EXTENSIONS);
+    validateFileSize(file, MAX_ORGUNIT_IMAGE_SIZE_IN_BYTES);
+  }
+
+  public void validateUserAvatar(MultipartFile file) {
+    validateContentType(file.getContentType(), ALLOWED_IMAGE_MIME_TYPES);
+    validateFileExtension(file.getOriginalFilename(), ALLOWED_IMAGE_FILE_EXTENSIONS);
+    validateFileSize(file, MAX_AVATAR_IMAGE_SIZE_IN_BYTES);
+  }
+
+  private void validateContentType(String contentType, List<String> validExtensions) {
+    if (contentType == null) {
+      throw new IllegalQueryException("Invalid content type, content type is NULL");
+    }
+    contentType = contentType.split(";")[0].trim();
+    if (!validExtensions.contains(contentType)) {
+      throw new IllegalQueryException(
+          "Invalid content type, valid content types are: " + String.join(",", validExtensions));
     }
+  }
 
-    @Override
-    public InputStream openStream() throws IOException {
-      try {
-        return file.getInputStream();
-      } catch (IOException ioe) {
-        return new NullInputStream(0);
+  public static void validateCustomIconFile(MultipartFile file) {
+    validateFileExtension(file.getOriginalFilename(), CUSTOM_ICON_VALID_ICON_EXTENSIONS);
+    validateFileSize(file, CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES);
+  }
+
+  private static void validateFileExtension(String fileName, List<String> validExtension) {
+    if (getExtension(fileName) == null || !validExtension.contains(getExtension(fileName))) {
+      throw new IllegalQueryException(
+          "Wrong file extension, valid extensions are: " + String.join(",", validExtension));
+    }
+  }
+
+  private static void validateFileSize(MultipartFile file, long maxFileSizeInBytes) {
+    if (file.getSize() > maxFileSizeInBytes) {
+      throw new IllegalQueryException(
+          String.format(
+              "File size can't be bigger than %d, current file size %d",
+              maxFileSizeInBytes, file.getSize()));
+    }
+  }
+
+  public static MultipartFile resizeImageToCustomSize(
+      MultipartFile multipartFile, int targetWidth, int targetHeight, Mode resizeMode)
+      throws IOException {
+    File tmpFile = null;
+
+    try {
+      BufferedImage resizedImage =
+          resize(
+              ImageIO.read(multipartFile.getInputStream()), resizeMode, targetWidth, targetHeight);
+
+      tmpFile = Files.createTempFile("org.hisp.dhis", ".tmp").toFile();
+
+      ImageIO.write(
+          resizedImage,
+          Objects.requireNonNull(getExtension(multipartFile.getOriginalFilename())),
+          tmpFile);
+
+      FileItem fileItem =
+          new DiskFileItemFactory()
+              .createItem(
+                  "file",
+                  Files.probeContentType(tmpFile.toPath()),
+                  false,
+                  multipartFile.getOriginalFilename());
+
+      try (InputStream in = new FileInputStream(tmpFile);
+          OutputStream out = fileItem.getOutputStream()) {
+        in.transferTo(out);
+      }
+
+      return new CommonsMultipartFile(fileItem);
+    } catch (Exception e) {
+      throw new IOException("Failed to resize image: " + e.getMessage());
+    } finally {
+      if (tmpFile != null && tmpFile.exists()) {
+        Files.delete(tmpFile.toPath());
       }
     }
   }
+
+  public static MultipartFile resizeIconToDefaultSize(MultipartFile multipartFile)
+      throws IOException {
+    return resizeImageToCustomSize(
+        multipartFile, CUSTOM_ICON_TARGET_WIDTH, CUSTOM_ICON_TARGET_HEIGHT, Mode.FIT_EXACT);
+  }
+
+  public static MultipartFile resizeAvatarToDefaultSize(MultipartFile multipartFile)
+      throws IOException {
+    return resizeImageToCustomSize(
+        multipartFile, AVATAR_TARGET_WIDTH, AVATAR_TARGET_HEIGHT, Mode.AUTOMATIC);
+  }
+
+  public MultipartFile resizeOrgToDefaultSize(MultipartFile multipartFile) throws IOException {
+    return resizeImageToCustomSize(
+        multipartFile, ORGUNIT_TARGET_WIDTH, ORGUNIT_TARGET_HEIGHT, Mode.AUTOMATIC);
+  }
 }