You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2020/12/07 18:50:12 UTC
[ofbiz-framework] 01/04: Fixed: Secure the uploads (OFBIZ-12080)
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
commit 4481f373ca45514c1e6fb86f1f1d2c6204f7a65a
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Sun Dec 6 18:47:12 2020 +0100
Fixed: Secure the uploads (OFBIZ-12080)
Handles audio and video formats supported by Tika.
Adds few new audio and video formats in seed data.
AFAIK there are no ways to embed a webshell in an audio or video file. So I did
not sophisticate the validation, just rely on Tika.
I have also fixed bugs in SecuredUpload: in isValidSvgFile and
isValidImageIncludingSvgFile
---
.../datamodel/data/seed/ContentSeedData.xml | 10 ++-
.../org/apache/ofbiz/security/SecuredUpload.java | 94 +++++++++++++++++-----
2 files changed, 82 insertions(+), 22 deletions(-)
diff --git a/applications/datamodel/data/seed/ContentSeedData.xml b/applications/datamodel/data/seed/ContentSeedData.xml
index fcaa664..54472c1 100644
--- a/applications/datamodel/data/seed/ContentSeedData.xml
+++ b/applications/datamodel/data/seed/ContentSeedData.xml
@@ -405,10 +405,15 @@ under the License.
<!-- audio mime types -->
<MimeType mimeTypeId="audio/basic" description="Basic Audio"/>
- <MimeType mimeTypeId="audio/mpeg" description="MPEG Audio"/>
+ <MimeType mimeTypeId="audio/mpeg" description="MP3 Audio"/>
+ <MimeType mimeTypeId="audio/mp4" description="MP4 Audio"/>
<MimeType mimeTypeId="audio/x-ms-wax" description="WAX Audio"/>
- <MimeType mimeTypeId="audio/ogg" description="OGG Audio"/>
<MimeType mimeTypeId="audio/wav" description="WAV Audio"/>
+ <MimeType mimeTypeId="audio/ogg" description="OGG Audio"/>
+ <MimeType mimeTypeId="audio/x-ogg" description="OGG Audio"/>
+ <MimeType mimeTypeId="audio/vorbis" description="Vorbis Audio"/>
+ <MimeType mimeTypeId="audio/x-flac" description="FLAC Audio"/>
+ <MimeType mimeTypeId="audio/flac" description="FLAC Audio"/>
<!-- image mime types -->
<MimeType mimeTypeId="image/jpeg" description="JPEG/JPG Image"/>
@@ -465,6 +470,7 @@ under the License.
<MimeType mimeTypeId="video/x-ms-wm" description="WM Video"/>
<MimeType mimeTypeId="video/x-ms-wmv" description="WMV Video"/>
<MimeType mimeTypeId="video/x-ms-wmx" description="WMX Video"/>
+ <MimeType mimeTypeId="video/3gpp" description="3GP Mobile Video"/>
<FileExtension fileExtensionId="asf" mimeTypeId="video/x-ms-asf"/>
<FileExtension fileExtensionId="asx" mimeTypeId="video/x-ms-asf"/>
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 4650dfd..e233228 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
@@ -151,15 +151,23 @@ public class SecuredUpload {
}
break;
- // case "Audio": TODO if needed
- // break;
- // case "Video": TODO if needed
- // break;
+ case "Audio":
+ if (isValidAudioFile(fileTocheck)) {
+ return true;
+ }
+ break;
+ case "Video":
+ if (isValidVideoFile(fileTocheck)) {
+ return true;
+ }
+ break;
default: // All
if (isValidTextFile(fileTocheck)
|| isValidImageIncludingSvgFile(fileTocheck)
|| isValidCompressedFile(fileTocheck, delegator)
+ || isValidAudioFile(fileTocheck)
+ || isValidVideoFile(fileTocheck)
|| isValidPdfFile(fileTocheck)) {
return true;
}
@@ -299,14 +307,7 @@ public class SecuredUpload {
* @throws IOException ImageReadException
*/
private static boolean isValidImageIncludingSvgFile(String fileName) throws ImageReadException, IOException {
- Path filePath = Paths.get(fileName);
- byte[] bytesFromFile = Files.readAllBytes(filePath);
- ImageFormat imageFormat = Imaging.guessFormat(bytesFromFile);
- return imageFormat.equals(ImageFormats.PNG)
- || imageFormat.equals(ImageFormats.GIF)
- || imageFormat.equals(ImageFormats.TIFF)
- || imageFormat.equals(ImageFormats.JPEG)
- || isValidSvgFile(fileName);
+ return isValidImageFile(fileName) || isValidSvgFile(fileName);
}
/**
@@ -316,15 +317,19 @@ public class SecuredUpload {
* @throws IOException
*/
private static boolean isValidSvgFile(String fileName) throws IOException {
- Path filePath = Paths.get(fileName);
- String parser = XMLResourceDescriptor.getXMLParserClassName();
- SAXSVGDocumentFactory f = new SAXSVGDocumentFactory(parser);
- try {
- f.createDocument(filePath.toUri().toString());
- } catch (IOException e) {
- return false;
+ String mimeType = getMimeTypeFromFileName(fileName);
+ if ("image/svg+xml".equals(mimeType)) {
+ Path filePath = Paths.get(fileName);
+ String parser = XMLResourceDescriptor.getXMLParserClassName();
+ SAXSVGDocumentFactory f = new SAXSVGDocumentFactory(parser);
+ try {
+ f.createDocument(filePath.toUri().toString());
+ } catch (IOException e) {
+ return false;
+ }
+ return isValidTextFile(fileName); // Validate content to prevent webshell
}
- return isValidTextFile(fileName);
+ return false;
}
/**
@@ -501,6 +506,55 @@ public class SecuredUpload {
}
/**
+ * Is this a valid Audio file?
+ * @param fileName must be an UTF-8 encoded text file
+ * @return true if it's a valid Audio file?
+ * @throws IOException
+ */
+ private static boolean isValidAudioFile(String fileName) throws IOException {
+ String mimeType = getMimeTypeFromFileName(fileName);
+ if ("audio/basic".equals(mimeType)
+ || "audio/wav".equals(mimeType)
+ || "audio/x-ms-wax".equals(mimeType)
+ || "audio/mpeg".equals(mimeType)
+ || "audio/mp4".equals(mimeType)
+ || "audio/ogg".equals(mimeType)
+ || "audio/vorbis".equals(mimeType)
+ || "audio/x-ogg".equals(mimeType)
+ || "audio/flac".equals(mimeType)
+ || "audio/x-flac".equals(mimeType)) {
+ return true;
+ }
+ Debug.logError("The file" + fileName + " is not a valid audio file, for security reason it's not accepted :", MODULE);
+ return false;
+ }
+
+ /**
+ * Is this a valid Audio file?
+ * @param fileName must be an UTF-8 encoded text file
+ * @return true if it's a valid Audio file?
+ * @throws IOException
+ */
+ private static boolean isValidVideoFile(String fileName) throws IOException {
+ String mimeType = getMimeTypeFromFileName(fileName);
+ if ("video/avi".equals(mimeType)
+ || "video/mpeg".equals(mimeType)
+ || "video/mp4".equals(mimeType)
+ || "video/quicktime".equals(mimeType)
+ || "video/3gpp".equals(mimeType)
+ || "video/x-ms-asf".equals(mimeType)
+ || "video/x-flv".equals(mimeType)
+ || "video/x-ms-wvx".equals(mimeType)
+ || "video/x-ms-wm".equals(mimeType)
+ || "video/x-ms-wmv".equals(mimeType)
+ || "video/x-ms-wmx".equals(mimeType)) {
+ return true;
+ }
+ Debug.logError("The file" + fileName + " is not a valid video file, for security reason it's not accepted :", MODULE);
+ return false;
+ }
+
+ /**
* 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
* @return true if the text file does not contains a Freemarker SSTI