OFBiz › OFBiz - Commits

[ofbiz-framework] branch release18.12 updated (1b9784d -> b25776d)

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

3 messages Options

jleroux@apache.org

[ofbiz-framework] branch release18.12 updated (1b9784d -> b25776d)

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

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

from 1b9784d Fixed: Secure the uploads (OFBIZ-12080)
new 80709b7 Fixed: Secure the uploads (OFBIZ-12080)
new b25776d Fixed: Label issue on List Companies page (OFBIZ-12023)

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:
applications/accounting/widget/GlSetupForms.xml | 8 +-
.../datamodel/data/seed/ContentSeedData.xml | 10 ++-
.../org/apache/ofbiz/security/SecuredUpload.java | 94 +++++++++++++++++-----
3 files changed, 86 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 release18.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 80709b7da90ec5d43c24d4f615ec0ff51f626a83
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 fcaa664..54472c1 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: Fixed: Label issue on List Companies page (OFBIZ-12023)

In reply to this post by jleroux@apache.org