Skip to content

Commit 7b0e948

Browse files
author
drighetto
committed
Add method to check an image #1
1 parent 03b918e commit 7b0e948

12 files changed

+92
-0
lines changed

pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@
6464
<artifactId>javax.json</artifactId>
6565
<version>1.1</version>
6666
</dependency>
67+
<dependency>
68+
<groupId>org.apache.commons</groupId>
69+
<artifactId>commons-imaging</artifactId>
70+
<version>1.0.0-alpha5</version>
71+
</dependency>
6772
<!-- TEST ONLY PURPOSE -->
6873
<dependency>
6974
<groupId>org.junit.jupiter</groupId>

src/main/java/eu/righettod/SecurityUtils.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33

44
import org.apache.commons.csv.CSVFormat;
55
import org.apache.commons.csv.CSVRecord;
6+
import org.apache.commons.imaging.ImageInfo;
7+
import org.apache.commons.imaging.Imaging;
8+
import org.apache.commons.imaging.common.ImageMetadata;
69
import org.apache.commons.validator.routines.InetAddressValidator;
710
import org.apache.pdfbox.Loader;
811
import org.apache.pdfbox.pdmodel.PDDocument;
@@ -36,6 +39,7 @@
3639
import javax.xml.parsers.DocumentBuilder;
3740
import javax.xml.parsers.DocumentBuilderFactory;
3841
import javax.xml.parsers.ParserConfigurationException;
42+
import java.awt.image.BufferedImage;
3943
import java.io.*;
4044
import java.net.Inet6Address;
4145
import java.net.InetAddress;
@@ -849,4 +853,63 @@ public static boolean isJSONSafe(String json, int maxItemsByArraysCount, int max
849853
}
850854
return isSafe;
851855
}
856+
857+
/**
858+
* Apply a collection of validations on a image file provided:<br>
859+
* - Real image file.<br>
860+
* - Its mime type is into the list of allowed mime types.<br>
861+
* - Its metadata fields do not contains any characters related to a malicious payloads.<br>
862+
*
863+
* <br><br>
864+
* <b>Important note:</b>This implementation is prone to bypass using the "<b>raw insertion</b>" method documented in the <a href="https://www.synacktiv.com/en/publications/persistent-php-payloads-in-pngs-how-to-inject-php-code-in-an-image-and-keep-it-there">blog post</a> from the Synacktiv team.<br>
865+
* To handle such case, it is recommended to resize the image to remove any non image-related content, see <a href="https://github.com/righettod/document-upload-protection/blob/master/src/main/java/eu/righettod/poc/sanitizer/ImageDocumentSanitizerImpl.java#L54">here</a> for an example.<br>
866+
*
867+
* @param imageFilePath Filename of the image file to check.
868+
* @param imageAllowedMimeTypes List of image mime types allowed.
869+
* @return True only if the file pass all validations.
870+
* @see "https://commons.apache.org/proper/commons-imaging/"
871+
* @see "https://commons.apache.org/proper/commons-imaging/formatsupport.html"
872+
* @see "https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types"
873+
* @see "https://www.iana.org/assignments/media-types/media-types.xhtml#image"
874+
* @see "https://www.synacktiv.com/en/publications/persistent-php-payloads-in-pngs-how-to-inject-php-code-in-an-image-and-keep-it-there"
875+
* @see "https://cheatsheetseries.owasp.org/cheatsheets/File_Upload_Cheat_Sheet.html"
876+
* @see "https://github.com/righettod/document-upload-protection/blob/master/src/main/java/eu/righettod/poc/sanitizer/ImageDocumentSanitizerImpl.java"
877+
* @see "https://exiftool.org/examples.html"
878+
* @see "https://en.wikipedia.org/wiki/List_of_file_signatures"
879+
* @see "https://hexed.it/"
880+
* @see "https://github.com/sighook/pixload"
881+
*/
882+
public static boolean isImageSafe(String imageFilePath, List<String> imageAllowedMimeTypes) {
883+
boolean isSafe = false;
884+
Pattern payloadDetectionRegex = Pattern.compile("[<>${}`]+", Pattern.CASE_INSENSITIVE);
885+
try {
886+
File imgFile = new File(imageFilePath);
887+
if (imgFile.exists() && imgFile.canRead() && imgFile.isFile() && !imageAllowedMimeTypes.isEmpty()) {
888+
final byte[] imgBytes = Files.readAllBytes(imgFile.toPath());
889+
//Step 1: Check the mime type of the file against the allowed ones
890+
ImageInfo imgInfo = Imaging.getImageInfo(imgBytes);
891+
if (imageAllowedMimeTypes.contains(imgInfo.getMimeType())) {
892+
//Step 2: Load the image into an object using the Image API
893+
BufferedImage imgObject = Imaging.getBufferedImage(imgBytes);
894+
if (imgObject != null && imgObject.getWidth() > 0 && imgObject.getHeight() > 0) {
895+
//Step 3: Check the metadata if the image format support it - Highly experimental
896+
List<String> metadataWithPayloads = new ArrayList<>();
897+
final ImageMetadata imgMetadata = Imaging.getMetadata(imgBytes);
898+
if (imgMetadata != null) {
899+
imgMetadata.getItems().forEach(item -> {
900+
String metadata = item.toString();
901+
if (payloadDetectionRegex.matcher(metadata).find()) {
902+
metadataWithPayloads.add(metadata);
903+
}
904+
});
905+
}
906+
isSafe = metadataWithPayloads.isEmpty();
907+
}
908+
}
909+
}
910+
} catch (Exception e) {
911+
isSafe = false;
912+
}
913+
return isSafe;
914+
}
852915
}

src/test/java/eu/righettod/TestSecurityUtils.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,5 +384,29 @@ public void isJSONSafe() {
384384
}
385385
});
386386
}
387+
388+
@Test
389+
public void isImageSafe() {
390+
//Test safe image cases
391+
List<String> safeFileList = Arrays.asList("test-img-png-clean.png", "test-img-jpeg-clean.jpg", "test-img-gif-clean.gif");
392+
final List<String> imageAllowedMimeTypesCase1 = List.of("image/png", "image/jpeg", "image/gif");
393+
safeFileList.forEach(f -> {
394+
String testFile = getTestFilePath(f);
395+
assertTrue(SecurityUtils.isImageSafe(testFile, imageAllowedMimeTypesCase1), String.format(TEMPLATE_MESSAGE_FALSE_POSITIVE_FOR_FILE, testFile));
396+
});
397+
//Test safe image cases but with non allowed mime types
398+
final List<String> imageAllowedMimeTypesCase2 = List.of("image/emf");
399+
safeFileList.forEach(f -> {
400+
String testFile = getTestFilePath(f);
401+
assertFalse(SecurityUtils.isImageSafe(testFile, imageAllowedMimeTypesCase2), String.format(TEMPLATE_MESSAGE_FALSE_NEGATIVE_FOR_FILE, testFile));
402+
});
403+
//Test unsafe image cases
404+
List<String> unsafeFileList = Arrays.asList("test-img-png-phpcode-in-comments.png", "test-img-jpeg-cmd-in-artist.jpg", "test-img-jpeg-cmd-in-software.jpg", "test-img-exe-with-png-magicbytes.png", "test-img-gif-xsspayload-inserted-with-sighook-pixload.gif");
405+
final List<String> imageAllowedMimeTypesCase3 = List.of("image/png", "image/jpeg", "image/gif");
406+
unsafeFileList.forEach(f -> {
407+
String testFile = getTestFilePath(f);
408+
assertFalse(SecurityUtils.isImageSafe(testFile, imageAllowedMimeTypesCase3), String.format(TEMPLATE_MESSAGE_FALSE_NEGATIVE_FOR_FILE, testFile));
409+
});
410+
}
387411
}
388412

Loading
39.4 KB
Loading
Loading
50.7 KB
Loading
Loading
Loading
39.9 KB
Loading
Loading
Loading

0 commit comments

Comments
 (0)