This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 69cf43d78d Improved: Improve ImageManagementServices code (OFBIZ-13292)
69cf43d78d is described below
commit 69cf43d78defedb01b60a01c2f90cadd00211d18
Author: Jacques Le Roux <[email protected]>
AuthorDate: Wed Sep 24 09:07:51 2025 +0200
Improved: Improve ImageManagementServices code (OFBIZ-13292)
Better handling of PNG files
---
dependencies.gradle | 1 +
.../org/apache/ofbiz/security/SecuredUpload.java | 134 ++++++++++++++++++++-
2 files changed, 131 insertions(+), 4 deletions(-)
diff --git a/dependencies.gradle b/dependencies.gradle
index ba726ec72c..d52f7eb7af 100644
--- a/dependencies.gradle
+++ b/dependencies.gradle
@@ -17,6 +17,7 @@
* under the License.
*/
dependencies {
+ implementation 'com.drewnoakes:metadata-extractor:2.19.0'
implementation 'com.github.ben-manes.caffeine:caffeine:3.1.8'
implementation 'com.google.guava:guava:33.3.1-jre'
implementation 'com.google.zxing:core:3.5.3'
diff --git
a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
index cfe6e1f9ee..2cc62dbba8 100644
---
a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
+++
b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
@@ -23,6 +23,7 @@ import java.awt.Graphics;
import java.awt.Image;
import java.awt.Transparency;
import java.awt.image.BufferedImage;
+import java.io.DataInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
@@ -39,13 +40,18 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Base64;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
+import java.util.zip.DataFormatException;
+import java.util.zip.Inflater;
import javax.imageio.ImageIO;
import javax.imageio.ImageReader;
@@ -96,6 +102,10 @@ import org.mustangproject.ZUGFeRD.ZUGFeRDImporter;
import org.w3c.dom.Document;
import org.xml.sax.SAXException;
+import com.drew.imaging.ImageMetadataReader;
+import com.drew.imaging.ImageProcessingException;
+import com.drew.metadata.Directory;
+import com.drew.metadata.Tag;
import com.lowagie.text.pdf.PdfReader;
public class SecuredUpload {
@@ -255,7 +265,7 @@ public class SecuredUpload {
* @throws ImageReadException
*/
public static boolean isValidFile(String fileToCheck, String fileType,
Delegator delegator) throws IOException, ImageReadException {
- // Allow all
+ // Allow all uploads w/o check
if
(("true".equalsIgnoreCase(EntityUtilProperties.getPropertyValue("security",
"allowAllUploads", delegator)))) {
return true;
}
@@ -398,9 +408,17 @@ public class SecuredUpload {
*/
private static boolean imageMadeSafe(String fileName) {
File file = new File(fileName);
- boolean safeState = false;
boolean fallbackOnApacheCommonsImaging;
+ if (!noWebshellInMetadata(file)) {
+ return false;
+ }
+ if (!noWebshellInPNG(file)) {
+ return false;
+ }
+
+ boolean safeState = false;
+
if ((file != null) && file.exists() && file.canRead() &&
file.canWrite()) {
try (OutputStream fos = Files.newOutputStream(file.toPath(),
StandardOpenOption.WRITE)) {
// Get the image format
@@ -486,6 +504,114 @@ public class SecuredUpload {
return safeState;
}
+ private static boolean noWebshellInMetadata(File file) {
+ com.drew.metadata.Metadata metadata = null;
+ try {
+ metadata = ImageMetadataReader.readMetadata(file);
+ } catch (ImageProcessingException | IOException error) {
+ Debug.logError("================== Not saved for security reason
==================" + error, MODULE);
+ }
+
+ for (Directory directory : metadata.getDirectories()) {
+ for (Tag tag : directory.getTags()) {
+ try {
+ if (!isValidText(tag.toString(), Collections.emptyList()))
{
+ Debug.logError("================== Not saved for
security reason ==================", MODULE);
+ return false;
+ }
+ } catch (IOException error) {
+ Debug.logError("================== Not saved for security
reason ==================" + error, MODULE);
+ return false;
+ }
+ }
+ for (String error : directory.getErrors()) {
+ Debug.logError("================== Not saved for security
reason ==================" + error, MODULE);
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean noWebshellInPNG(File file) {
+ try {
+ ImageIO.read(file);
+ if (!isPNG(file)) {
+ return true; // Not a PNG file, it's OK so far
+ }
+ } catch (IOException error) {
+ Debug.logError("================== Not saved for security reason
==================" + error, MODULE);
+ return false;
+ }
+
+ try (DataInputStream stream = new DataInputStream(new
FileInputStream(file));) {
+ byte[] data = new byte[8];
+ stream.readFully(data); //Read PNG Header
+ while (true) {
+ data = new byte[4];
+ stream.readFully(data); //Read Length
+ int length = ((data[0] & 0xFF) << 24)
+ | ((data[1] & 0xFF) << 16)
+ | ((data[2] & 0xFF) << 8)
+ | (data[3] & 0xFF); //Byte array to int
+ stream.readFully(data); //Read Name
+ String name = new String(data); //Byte array to String
+ if (name.equals("IDAT")) {
+ data = new byte[length];
+ stream.readFully(data); //Read Data
+ return inflate(data);
+ } else { //Don't care about other chunks
+ data = new byte[length + 4]; //Data length + 4 byte CRC
+ stream.readFully(data); //Skip Data and CRC.
+ }
+ }
+ } catch (IOException error) {
+ Debug.logError("================== Not saved for security reason,
wrong PNG IDAT (weird) ==================" + error, MODULE);
+ return false;
+ }
+ }
+
+ private static boolean isPNG(File file) throws IOException {
+ Path filePath = Paths.get(file.getPath());
+ byte[] bytesFromFile = Files.readAllBytes(filePath);
+ ImageFormat imageFormat = Imaging.guessFormat(bytesFromFile);
+ return (imageFormat.equals(ImageFormats.PNG));
+ }
+
+ private static boolean inflate(byte[] data) {
+ Inflater inflater = new Inflater();
+ inflater.setInput(data);
+ byte[] result = new byte[data.length * 5]; // Inflating ratio max is
5/1
+ try {
+ while (!inflater.finished()) {
+ int count = inflater.inflate(result);
+ if (count == 0) {
+ if (!inflater.needsInput()) { // Not everything read
+ inflater.inflate(result);
+ } else if (inflater.needsDictionary()) { // Dictionary to
be loaded
+ inflater.setDictionary(result);
+ inflater.getAdler();
+ }
+ }
+ }
+ if (inflater.getRemaining() > 0) { // There is more than image
data in IDAT, check it
+ byte[] remaining = Arrays.copyOfRange(data, (int)
inflater.getBytesRead(), (int) inflater.getBytesRead() +
inflater.getRemaining());
+ String toCheck = new String(remaining, "UTF-8");
+ byte[] decoded = Base64.getDecoder().decode(toCheck);
+ String decodedStr = new String(decoded,
StandardCharsets.UTF_8);
+ if (!isValidText(decodedStr, Collections.emptyList())) {
+ Debug.logError("================== Not saved for security
reason ==================", MODULE);
+ inflater.end();
+ return false;
+ }
+ }
+ } catch (DataFormatException | IOException error) {
+ Debug.logError("================== Not saved for security reason
==================" + error, MODULE);
+ inflater.end();
+ return false;
+ }
+ return true;
+ }
+
/**
* Is it a supported image format, including SVG?
* @param fileName
@@ -840,8 +966,8 @@ public class SecuredUpload {
/**
* Does this text file contains a Freemarker Server-Side Template
Injection (SSTI) using freemarker.template.utility.Execute? Etc.
* @param fileName must be an UTF-8 encoded text file
- * @param encodedContent TODO
- * @return true if the text file does not contains a Freemarker SSTI
+ * @param encodedContent true id the file content is encoded
+ * @return true if the text file does not contains a Freemarker SSTI or
other issues
* @throws IOException
*/
private static boolean isValidTextFile(String fileName, Boolean
encodedContent) throws IOException {
