Validate fileName to prevent path traversal

Nuklon · Oct 29, 2021 · 56e0598 · 56e0598
1 parent c3625ae
commit 56e0598
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 29 deletions.
diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
@@ -323,6 +323,7 @@
     <key alias="orClickHereToUpload">or click here to choose files</key>
     <key alias="dragFilesHereToUpload">You can drag files here to upload</key>
     <key alias="disallowedFileType">Cannot upload this file, it does not have an approved file type</key>
+    <key alias="invalidFileName">Cannot upload this file, it does not have a valid file name</key>
     <key alias="maxFileSize">Max file size is</key>
     <key alias="mediaRoot">Media root</key>
     <key alias="moveFailed">Failed to move media</key>

diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
@@ -328,6 +328,7 @@
     <key alias="orClickHereToUpload">or click here to choose files</key>
         <key alias="dragFilesHereToUpload">You can drag files here to upload.</key>
     <key alias="disallowedFileType">Cannot upload this file, it does not have an approved file type</key>
+    <key alias="invalidFileName">Cannot upload this file, it does not have a valid file name</key>
     <key alias="maxFileSize">Max file size is</key>
     <key alias="mediaRoot">Media root</key>
     <key alias="moveFailed">Failed to move media</key>

diff --git a/src/Umbraco.Web/Editors/ContentTypeController.cs b/src/Umbraco.Web/Editors/ContentTypeController.cs
@@ -576,43 +576,53 @@ public async Task<ContentTypeImportModel> Upload()
             var fileName = file.Headers.ContentDisposition.FileName.Trim(Constants.CharArrays.DoubleQuote);
             var ext = fileName.Substring(fileName.LastIndexOf('.') + 1).ToLower();
 
-            var destFileName = root + "\\" + fileName;
-            try
-            {
-                // due to a bug before 8.7.0 we didn't delete temp files, so we need to make sure to delete before
-                // moving else you get errors and the upload fails without a message in the UI (there's a JS error)
-                if(System.IO.File.Exists(destFileName))
-                    System.IO.File.Delete(destFileName);
-
-                // renaming the file because MultipartFormDataStreamProvider has created a random fileName instead of using the name from the
-                // content-disposition for more than 6 years now. Creating a CustomMultipartDataStreamProvider deriving from MultipartFormDataStreamProvider
-                // seems like a cleaner option, but I'm not sure where to put it and renaming only takes one line of code.
-                System.IO.File.Move(result.FileData[0].LocalFileName, destFileName);
-            }
-            catch (Exception ex)
+            var destFileName = Path.Combine(root, fileName);
+            if (Path.GetFullPath(destFileName).StartsWith(Path.GetFullPath(root)))
             {
-                Logger.Error<ContentTypeController, string>(ex, "Error uploading udt file to App_Data: {File}", destFileName);
-            }
-
-            if (ext.InvariantEquals("udt"))
-            {
-                model.TempFileName = Path.Combine(root, fileName);
+                try
+                {
+                    // due to a bug before 8.7.0 we didn't delete temp files, so we need to make sure to delete before
+                    // moving else you get errors and the upload fails without a message in the UI (there's a JS error)
+                    if(System.IO.File.Exists(destFileName))
+                        System.IO.File.Delete(destFileName);
+
+                    // renaming the file because MultipartFormDataStreamProvider has created a random fileName instead of using the name from the
+                    // content-disposition for more than 6 years now. Creating a CustomMultipartDataStreamProvider deriving from MultipartFormDataStreamProvider
+                    // seems like a cleaner option, but I'm not sure where to put it and renaming only takes one line of code.
+                    System.IO.File.Move(result.FileData[0].LocalFileName, destFileName);
+                }
+                catch (Exception ex)
+                {
+                    Logger.Error<ContentTypeController, string>(ex, "Error uploading udt file to App_Data: {File}", destFileName);
+                }
 
-                var xd = new XmlDocument
+                if (ext.InvariantEquals("udt"))
                 {
-                    XmlResolver = null
-                };
-                xd.Load(model.TempFileName);
+                    model.TempFileName = destFileName;
+
+                    var xd = new XmlDocument
+                    {
+                        XmlResolver = null
+                    };
+                    xd.Load(model.TempFileName);
 
-                model.Alias = xd.DocumentElement?.SelectSingleNode("//DocumentType/Info/Alias")?.FirstChild.Value;
-                model.Name = xd.DocumentElement?.SelectSingleNode("//DocumentType/Info/Name")?.FirstChild.Value;
+                    model.Alias = xd.DocumentElement?.SelectSingleNode("//DocumentType/Info/Alias")?.FirstChild.Value;
+                    model.Name = xd.DocumentElement?.SelectSingleNode("//DocumentType/Info/Name")?.FirstChild.Value;
+                }
+                else
+                {
+                    model.Notifications.Add(new Notification(
+                        Services.TextService.Localize("speechBubbles", "operationFailedHeader"),
+                        Services.TextService.Localize("media", "disallowedFileType"),
+                        NotificationStyle.Warning));
+                }
             }
             else
             {
                 model.Notifications.Add(new Notification(
-                    Services.TextService.Localize("speechBubbles", "operationFailedHeader"),
-                    Services.TextService.Localize("media", "disallowedFileType"),
-                    NotificationStyle.Warning));
+                                        Services.TextService.Localize("speechBubbles", "operationFailedHeader"),
+                                        Services.TextService.Localize("media", "invalidFileName"),
+                                        NotificationStyle.Warning));
             }
 
             return model;