Skip to content

Commit 7505efd

Browse files
authored
Merge commit from fork
* Ensure imported dictionary item is only processed from the expected temporary uploads folder. * Ensured content type upload input is only a file and not a string that can be interpretted as a file path. * Amend dictionary import to extract file name and prepend path rather than rely on provided path. * Tidied usings. * Ensure file name cannot contain path separator characters.
1 parent 90ec853 commit 7505efd

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

‎src/Umbraco.Web.BackOffice/Controllers/ContentTypeController.cs‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -674,8 +674,21 @@ public IActionResult Export(int id)
674674
[Authorize(Policy=AuthorizationPolicies.TreeAccessDocumentTypes)]
675675
publicIActionResultImport(stringfile)
676676
{
677+
if(string.IsNullOrWhiteSpace(file))
678+
{
679+
returnNotFound();
680+
}
681+
682+
// The incoming 'file' parameter we expect to contain the just a file name and extension.
683+
// We accept only this and no input parameters containing paths, to prevent any path based security exploits.
684+
varinvalidFileNameChars=Path.GetInvalidFileNameChars();
685+
if(file.IndexOfAny(invalidFileNameChars)>=0||file.Contains(Path.DirectorySeparatorChar)||file.Contains(Path.AltDirectorySeparatorChar))
686+
{
687+
returnNotFound();
688+
}
689+
677690
varfilePath=Path.Combine(_hostingEnvironment.MapPathContentRoot(Constants.SystemDirectories.TempFileUploads),file);
678-
if(string.IsNullOrEmpty(file)||!System.IO.File.Exists(filePath))
691+
if(System.IO.File.Exists(filePath)isfalse)
679692
{
680693
returnNotFound();
681694
}

‎src/Umbraco.Web.BackOffice/Controllers/DictionaryController.cs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,27 @@
1-
usingSystem.Xml;
21
usingSystem.Globalization;
32
usingSystem.Net.Mime;
43
usingSystem.Text;
4+
usingSystem.Xml;
5+
usingSystem.Xml.Linq;
56
usingMicrosoft.AspNetCore.Authorization;
7+
usingMicrosoft.AspNetCore.Http;
68
usingMicrosoft.AspNetCore.Mvc;
9+
usingMicrosoft.Extensions.DependencyInjection;
710
usingMicrosoft.Extensions.Logging;
811
usingMicrosoft.Extensions.Options;
912
usingUmbraco.Cms.Core;
1013
usingUmbraco.Cms.Core.Configuration.Models;
1114
usingUmbraco.Cms.Core.DependencyInjection;
15+
usingUmbraco.Cms.Core.Hosting;
1216
usingUmbraco.Cms.Core.Mapping;
1317
usingUmbraco.Cms.Core.Models;
1418
usingUmbraco.Cms.Core.Models.ContentEditing;
15-
usingUmbraco.Cms.Core.Hosting;
1619
usingUmbraco.Cms.Core.Security;
1720
usingUmbraco.Cms.Core.Services;
21+
usingUmbraco.Cms.Infrastructure.Packaging;
1822
usingUmbraco.Cms.Web.Common.Attributes;
1923
usingUmbraco.Cms.Web.Common.Authorization;
2024
usingUmbraco.Extensions;
21-
usingUmbraco.Cms.Infrastructure.Packaging;
22-
usingSystem.Xml.Linq;
23-
usingMicrosoft.AspNetCore.Http;
24-
usingMicrosoft.Extensions.DependencyInjection;
2525

2626
namespaceUmbraco.Cms.Web.BackOffice.Controllers;
2727

@@ -460,7 +460,17 @@ public IActionResult ImportDictionary(string file, int parentId)
460460
returnNotFound();
461461
}
462462

463-
varfilePath=Path.Combine(_hostingEnvironment.MapPathContentRoot(Constants.SystemDirectories.Data),file);
463+
// The incoming 'file' parameter we expect to contain the full path to the uploaded file.
464+
// We accept only files coming from the uploads folder to prevent any path based security exploits.
465+
varfileName=Path.GetFileName(file);
466+
varinvalidFileNameChars=Path.GetInvalidFileNameChars();
467+
if(fileName.IndexOfAny(invalidFileNameChars)>=0||fileName.Contains(Path.DirectorySeparatorChar)||fileName.Contains(Path.AltDirectorySeparatorChar))
468+
{
469+
returnNotFound();
470+
}
471+
472+
varroot=_hostingEnvironment.MapPathContentRoot(Constants.SystemDirectories.TempFileUploads);
473+
varfilePath=Path.Combine(root,fileName);
464474
if(!System.IO.File.Exists(filePath))
465475
{
466476
returnNotFound();

0 commit comments

Comments
(0)