From 3d21448f894d6749c68f2e48fcad9f4d2accb022 Mon Sep 17 00:00:00 2001
From: AndyButland <abutland73@gmail.com>
Date: Tue, 9 May 2017 18:12:51 +0200
Subject: [PATCH] Validated file uploads using white list if provided, before
 falling back to blacklist

---
 .../Configuration/UmbracoSettings/ContentElement.cs   | 11 +++++++++++
 .../Configuration/UmbracoSettings/IContentSection.cs  |  4 +++-
 .../UmbracoSettings/ContentElementTests.cs            |  6 ++++++
 .../UmbracoSettings/umbracoSettings.config            |  3 +++
 .../PropertyEditors/UploadFileTypeValidator.cs        |  6 +++++-
 src/umbraco.businesslogic/UmbracoSettings.cs          |  8 ++++++++
 src/umbraco.editorControls/uploadfield/uploadField.cs |  7 +++++--
 7 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/ContentElement.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/ContentElement.cs
index 51a39e15df..2bab497b2b 100644
--- a/src/Umbraco.Core/Configuration/UmbracoSettings/ContentElement.cs
+++ b/src/Umbraco.Core/Configuration/UmbracoSettings/ContentElement.cs
@@ -139,6 +139,12 @@ namespace Umbraco.Core.Configuration.UmbracoSettings
         internal CommaDelimitedConfigurationElement DisallowedUploadFiles
         {
             get { return GetOptionalDelimitedElement("disallowedUploadFiles", new[] {"ashx", "aspx", "ascx", "config", "cshtml", "vbhtml", "asmx", "air", "axd"}); }
+        }
+
+        [ConfigurationProperty("allowedUploadFiles")]
+        internal CommaDelimitedConfigurationElement AllowedUploadFiles
+        {
+            get { return GetOptionalDelimitedElement("allowedUploadFiles", new string[0]); }
         }
 
         [ConfigurationProperty("cloneXmlContent")]
@@ -307,6 +313,11 @@ namespace Umbraco.Core.Configuration.UmbracoSettings
         IEnumerable<string> IContentSection.DisallowedUploadFiles
         {
             get { return DisallowedUploadFiles; }
+        }
+
+        IEnumerable<string> IContentSection.AllowedUploadFiles
+        {
+            get { return AllowedUploadFiles; }
         }
 
         bool IContentSection.CloneXmlContent
diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/IContentSection.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/IContentSection.cs
index d73b3b9e41..7e874c9582 100644
--- a/src/Umbraco.Core/Configuration/UmbracoSettings/IContentSection.cs
+++ b/src/Umbraco.Core/Configuration/UmbracoSettings/IContentSection.cs
@@ -52,7 +52,9 @@ namespace Umbraco.Core.Configuration.UmbracoSettings
 
         MacroErrorBehaviour MacroErrorBehaviour { get; }
         
-        IEnumerable<string> DisallowedUploadFiles { get; }
+        IEnumerable<string> DisallowedUploadFiles { get; }
+
+        IEnumerable<string> AllowedUploadFiles { get; }
 
         bool CloneXmlContent { get; }
 
diff --git a/src/Umbraco.Tests/Configurations/UmbracoSettings/ContentElementTests.cs b/src/Umbraco.Tests/Configurations/UmbracoSettings/ContentElementTests.cs
index 06dab42556..cb30b1f782 100644
--- a/src/Umbraco.Tests/Configurations/UmbracoSettings/ContentElementTests.cs
+++ b/src/Umbraco.Tests/Configurations/UmbracoSettings/ContentElementTests.cs
@@ -177,6 +177,12 @@ namespace Umbraco.Tests.Configurations.UmbracoSettings
         public void DisallowedUploadFiles()
         {
             Assert.IsTrue(SettingsSection.Content.DisallowedUploadFiles.All(x => "ashx,aspx,ascx,config,cshtml,vbhtml,asmx,air,axd".Split(',').Contains(x)));
+        }
+
+        [Test]
+        public void AllowedUploadFiles()
+        {
+            Assert.IsTrue(SettingsSection.Content.AllowedUploadFiles.All(x => "jpg,gif,png".Split(',').Contains(x)));
         }
     }
 }
diff --git a/src/Umbraco.Tests/Configurations/UmbracoSettings/umbracoSettings.config b/src/Umbraco.Tests/Configurations/UmbracoSettings/umbracoSettings.config
index 80eaee77d3..a6f9826492 100644
--- a/src/Umbraco.Tests/Configurations/UmbracoSettings/umbracoSettings.config
+++ b/src/Umbraco.Tests/Configurations/UmbracoSettings/umbracoSettings.config
@@ -100,6 +100,9 @@
     <!-- These file types will not be allowed to be uploaded via the upload control for media and content -->
     <disallowedUploadFiles>ashx,aspx,ascx,config,cshtml,vbhtml,asmx,air,axd</disallowedUploadFiles>
 
+    <!-- If completed, only the file extensions listed below will be allowed to be uploaded.  If empty, disallowedUploadFiles will apply to prevent upload of specific file extensions. -->
+    <allowedUploadFiles>jpg,png,gif</allowedUploadFiles>    
+
     <!-- Defines the default document type property used when adding properties in the back-office (if missing or empty, defaults to Textstring -->
     <defaultDocumentTypeProperty>Textstring</defaultDocumentTypeProperty>
   </content>
diff --git a/src/Umbraco.Web/PropertyEditors/UploadFileTypeValidator.cs b/src/Umbraco.Web/PropertyEditors/UploadFileTypeValidator.cs
index 96a1211589..5bc1df6bc4 100644
--- a/src/Umbraco.Web/PropertyEditors/UploadFileTypeValidator.cs
+++ b/src/Umbraco.Web/PropertyEditors/UploadFileTypeValidator.cs
@@ -42,7 +42,11 @@ namespace Umbraco.Web.PropertyEditors
         {
             if (fileName.IndexOf('.') <= 0) return false;
             var extension = Path.GetExtension(fileName).TrimStart(".");
-            return UmbracoConfig.For.UmbracoSettings().Content.DisallowedUploadFiles.Any(x => StringExtensions.InvariantEquals(x, extension)) == false;
+            
+            // Is valid if extension is whitelisted OR if there is no whitelist and extension is NOT blacklisted
+            return UmbracoConfig.For.UmbracoSettings().Content.AllowedUploadFiles.Any(x => x.InvariantEquals(extension)) ||
+                (UmbracoConfig.For.UmbracoSettings().Content.AllowedUploadFiles.Any() == false &&
+                UmbracoConfig.For.UmbracoSettings().Content.DisallowedUploadFiles.Any(x => x.InvariantEquals(extension)) == false);
         }
 
     }
diff --git a/src/umbraco.businesslogic/UmbracoSettings.cs b/src/umbraco.businesslogic/UmbracoSettings.cs
index d73ea22844..dea58ab3ae 100644
--- a/src/umbraco.businesslogic/UmbracoSettings.cs
+++ b/src/umbraco.businesslogic/UmbracoSettings.cs
@@ -270,6 +270,14 @@ namespace umbraco
         public static IEnumerable<string> DisallowedUploadFiles
         {
             get { return UmbracoConfig.For.UmbracoSettings().Content.DisallowedUploadFiles; }
+        }
+
+        /// <summary>
+        /// File types that will be allowed to be uploaded via the content/media upload control
+        /// </summary>
+        public static IEnumerable<string> AllowedUploadFiles
+        {
+            get { return UmbracoConfig.For.UmbracoSettings().Content.AllowedUploadFiles; }
         }
 
         /// <summary>
diff --git a/src/umbraco.editorControls/uploadfield/uploadField.cs b/src/umbraco.editorControls/uploadfield/uploadField.cs
index 6f795e1ad9..9c2d26bbd4 100644
--- a/src/umbraco.editorControls/uploadfield/uploadField.cs
+++ b/src/umbraco.editorControls/uploadfield/uploadField.cs
@@ -90,8 +90,11 @@ namespace umbraco.editorControls
 
             //now check the file type
             var extension = Path.GetExtension(postedFile.FileName).TrimStart(".");
-
-            return UmbracoConfig.For.UmbracoSettings().Content.DisallowedUploadFiles.Any(x => x.InvariantEquals(extension)) == false;
+
+            // allow if extension is whitelisted OR if there is no whitelist and extension is NOT blacklisted
+            return UmbracoConfig.For.UmbracoSettings().Content.AllowedUploadFiles.Any(x => x.InvariantEquals(extension)) ||
+                (UmbracoConfig.For.UmbracoSettings().Content.AllowedUploadFiles.Any() == false &&
+                UmbracoConfig.For.UmbracoSettings().Content.DisallowedUploadFiles.Any(x => x.InvariantEquals(extension)) == false);
         }
 
         public string Text