OFBiz › OFBiz - Commits

[ofbiz-framework] branch release17.12 updated (dc57528 -> e50ad56)

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

3 messages Options

jleroux@apache.org

[ofbiz-framework] branch release17.12 updated (dc57528 -> e50ad56)

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a change to branch release17.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.

from dc57528 Fixed: Secure the uploads (OFBIZ-12080)
new 7ff8fb8 Fixed: Secure the uploads (OFBIZ-12080)
new e50ad56 Improved: Adds the HTML <input> accept Attribute in form widgets and Freemaker templates (OFBIZ-12049)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

Summary of changes:
.../content/template/website/WebSiteCMSContent.ftl | 19 ++++-
.../datamodel/data/seed/ContentSeedData.xml | 15 +++-
.../org/apache/ofbiz/security/SecuredUpload.java | 94 +++++++++++++++++-----
3 files changed, 102 insertions(+), 26 deletions(-)

jleroux@apache.org

[ofbiz-framework] 01/02: Fixed: Secure the uploads (OFBIZ-12080)

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch release17.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 7ff8fb814e6ab5fed1fba39764f19b55ac4c4c05
Author: Jacques Le Roux <[hidden email]>
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 12106d6..a7ca706 100644
--- a/applications/datamodel/data/seed/ContentSeedData.xml
+++ b/applications/datamodel/data/seed/ContentSeedData.xml
@@ -405,10 +405,15 @@ under the License.


<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"/>


<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 3fdae3d..6247453 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

jleroux@apache.org

[ofbiz-framework] 02/02: Improved: Adds the HTML <input> accept Attribute in form widgets and Freemaker templates (OFBIZ-12049)

In reply to this post by jleroux@apache.org