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 <jacques.le.r...@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