Merge remote-tracking branch 'origin/v10/dev' into v11/dev

This commit is contained in:
Nikolaj
2023-08-22 10:24:26 +02:00
15 changed files with 464 additions and 92 deletions

View File

@@ -326,6 +326,9 @@ namespace Umbraco.Cms.Core.DependencyInjection
Services.AddUnique<ICultureImpactFactory>(provider => new CultureImpactFactory(provider.GetRequiredService<IOptionsMonitor<ContentSettings>>()));
Services.AddUnique<IDictionaryService, DictionaryService>();
Services.AddUnique<ITemporaryMediaService, TemporaryMediaService>();
// Register filestream security analyzers
Services.AddUnique<IFileStreamSecurityValidator,FileStreamSecurityValidator>();
}
}
}

View File

@@ -341,6 +341,7 @@
<key alias="createFolderFailed">Failed to create a folder under parent id %0%</key>
<key alias="renameFolderFailed">Failed to rename the folder with id %0%</key>
<key alias="dragAndDropYourFilesIntoTheArea">Drag and drop your file(s) into the area</key>
<key alias="fileSecurityValidationFailure">One or more file security validations have failed</key>
</area>
<area alias="member">
<key alias="createNewMember">Create a new member</key>

View File

@@ -352,6 +352,7 @@
<key alias="renameFolderFailed">Failed to rename the folder with id %0%</key>
<key alias="dragAndDropYourFilesIntoTheArea">Drag and drop your file(s) into the area</key>
<key alias="uploadNotAllowed">Upload is not allowed in this location.</key>
<key alias="fileSecurityValidationFailure">One or more file security validations have failed</key>
</area>
<area alias="member">
<key alias="createNewMember">Create a new member</key>

View File

@@ -340,6 +340,7 @@
<key alias="renameFolderFailed">Kan de map met id %0% niet hernoemen</key>
<key alias="dragAndDropYourFilesIntoTheArea">Sleep en zet je bestand(en) neer in dit gebied</key>
<key alias="uploadNotAllowed">Upload is niet toegelaten in deze locatie.</key>
<key alias="fileSecurityValidationFailure">Een of meerdere veiligheid validaties zijn gefaald voor het bestand</key>
</area>
<area alias="member">
<key alias="createNewMember">Maak nieuw lid aan</key>

View File

@@ -219,96 +219,52 @@ public static class TypeExtensions
/// <returns></returns>
public static PropertyInfo[] GetAllProperties(this Type type)
{
if (type.IsInterface)
{
var propertyInfos = new List<PropertyInfo>();
var considered = new List<Type>();
var queue = new Queue<Type>();
considered.Add(type);
queue.Enqueue(type);
while (queue.Count > 0)
{
Type subType = queue.Dequeue();
foreach (Type subInterface in subType.GetInterfaces())
{
if (considered.Contains(subInterface))
{
continue;
}
considered.Add(subInterface);
queue.Enqueue(subInterface);
}
PropertyInfo[] typeProperties = subType.GetProperties(
BindingFlags.FlattenHierarchy
| BindingFlags.Public
| BindingFlags.NonPublic
| BindingFlags.Instance);
IEnumerable<PropertyInfo> newPropertyInfos = typeProperties
.Where(x => !propertyInfos.Contains(x));
propertyInfos.InsertRange(0, newPropertyInfos);
}
return propertyInfos.ToArray();
}
return type.GetProperties(BindingFlags.FlattenHierarchy
| BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
const BindingFlags bindingFlags = BindingFlags.FlattenHierarchy
| BindingFlags.Public
| BindingFlags.NonPublic
| BindingFlags.Instance;
return type.GetAllMemberInfos(t => t.GetProperties(bindingFlags));
}
/// <summary>
/// Returns all public properties including inherited properties even for interfaces
/// Returns public properties including inherited properties even for interfaces
/// </summary>
/// <param name="type"></param>
/// <returns></returns>
/// <remarks>
/// taken from
/// http://stackoverflow.com/questions/358835/getproperties-to-return-all-properties-for-an-interface-inheritance-hierarchy
/// </remarks>
public static PropertyInfo[] GetPublicProperties(this Type type)
{
if (type.IsInterface)
{
var propertyInfos = new List<PropertyInfo>();
const BindingFlags bindingFlags = BindingFlags.FlattenHierarchy
| BindingFlags.Public
| BindingFlags.Instance;
return type.GetAllMemberInfos(t => t.GetProperties(bindingFlags));
}
var considered = new List<Type>();
var queue = new Queue<Type>();
considered.Add(type);
queue.Enqueue(type);
while (queue.Count > 0)
{
Type subType = queue.Dequeue();
foreach (Type subInterface in subType.GetInterfaces())
{
if (considered.Contains(subInterface))
{
continue;
}
/// <summary>
/// Returns public methods including inherited methods even for interfaces
/// </summary>
/// <param name="type"></param>
/// <returns></returns>
public static MethodInfo[] GetPublicMethods(this Type type)
{
const BindingFlags bindingFlags = BindingFlags.FlattenHierarchy
| BindingFlags.Public
| BindingFlags.Instance;
return type.GetAllMemberInfos(t => t.GetMethods(bindingFlags));
}
considered.Add(subInterface);
queue.Enqueue(subInterface);
}
PropertyInfo[] typeProperties = subType.GetProperties(
BindingFlags.FlattenHierarchy
| BindingFlags.Public
| BindingFlags.Instance);
IEnumerable<PropertyInfo> newPropertyInfos = typeProperties
.Where(x => !propertyInfos.Contains(x));
propertyInfos.InsertRange(0, newPropertyInfos);
}
return propertyInfos.ToArray();
}
return type.GetProperties(BindingFlags.FlattenHierarchy
| BindingFlags.Public | BindingFlags.Instance);
/// <summary>
/// Returns all methods including inherited methods even for interfaces
/// </summary>
/// <remarks>Includes both Public and Non-Public methods</remarks>
/// <param name="type"></param>
/// <returns></returns>
public static MethodInfo[] GetAllMethods(this Type type)
{
const BindingFlags bindingFlags = BindingFlags.FlattenHierarchy
| BindingFlags.Public
| BindingFlags.NonPublic
| BindingFlags.Instance;
return type.GetAllMemberInfos(t => t.GetMethods(bindingFlags));
}
/// <summary>
@@ -512,4 +468,47 @@ public static class TypeExtensions
return attempt;
}
/// <remarks>
/// taken from
/// http://stackoverflow.com/questions/358835/getproperties-to-return-all-properties-for-an-interface-inheritance-hierarchy
/// </remarks>
private static T[] GetAllMemberInfos<T>(this Type type, Func<Type, T[]> getMemberInfos)
where T : MemberInfo
{
if (type.IsInterface is false)
{
return getMemberInfos(type);
}
var memberInfos = new List<T>();
var considered = new List<Type>();
var queue = new Queue<Type>();
considered.Add(type);
queue.Enqueue(type);
while (queue.Count > 0)
{
Type subType = queue.Dequeue();
foreach (Type subInterface in subType.GetInterfaces())
{
if (considered.Contains(subInterface))
{
continue;
}
considered.Add(subInterface);
queue.Enqueue(subInterface);
}
T[] typeMethodInfos = getMemberInfos(subType);
IEnumerable<T> newMethodInfos = typeMethodInfos
.Where(x => !memberInfos.Contains(x));
memberInfos.InsertRange(0, newMethodInfos);
}
return memberInfos.ToArray();
}
}

View File

@@ -0,0 +1,38 @@
namespace Umbraco.Cms.Core.Security;
public class FileStreamSecurityValidator : IFileStreamSecurityValidator
{
private readonly IEnumerable<IFileStreamSecurityAnalyzer> _fileAnalyzers;
public FileStreamSecurityValidator(IEnumerable<IFileStreamSecurityAnalyzer> fileAnalyzers)
{
_fileAnalyzers = fileAnalyzers;
}
/// <summary>
/// Analyzes whether the file content is considered safe with registered IFileStreamSecurityAnalyzers
/// </summary>
/// <param name="fileStream">Needs to be a Read seekable stream</param>
/// <returns>Whether the file is considered safe after running the necessary analyzers</returns>
public bool IsConsideredSafe(Stream fileStream)
{
foreach (var fileAnalyzer in _fileAnalyzers)
{
fileStream.Seek(0, SeekOrigin.Begin);
if (!fileAnalyzer.ShouldHandle(fileStream))
{
continue;
}
fileStream.Seek(0, SeekOrigin.Begin);
if (fileAnalyzer.IsConsideredSafe(fileStream) == false)
{
return false;
}
}
fileStream.Seek(0, SeekOrigin.Begin);
// If no analyzer we consider the file to be safe as the implementer has the possibility to add additional analyzers
// Or all analyzers deem te file to be safe
return true;
}
}

View File

@@ -0,0 +1,20 @@
namespace Umbraco.Cms.Core.Security;
public interface IFileStreamSecurityAnalyzer
{
/// <summary>
/// Indicates whether the analyzer should process the file
/// The implementation should be considerably faster than IsConsideredSafe
/// </summary>
/// <param name="fileStream"></param>
/// <returns></returns>
bool ShouldHandle(Stream fileStream);
/// <summary>
/// Analyzes whether the file content is considered safe
/// </summary>
/// <param name="fileStream">Needs to be a Read/Write seekable stream</param>
/// <returns>Whether the file is considered safe</returns>
bool IsConsideredSafe(Stream fileStream);
}

View File

@@ -0,0 +1,11 @@
namespace Umbraco.Cms.Core.Security;
public interface IFileStreamSecurityValidator
{
/// <summary>
/// Analyzes wether the file content is considered safe with registered IFileStreamSecurityAnalyzers
/// </summary>
/// <param name="fileStream">Needs to be a Read seekable stream</param>
/// <returns>Whether the file is considered safe after running the necessary analyzers</returns>
bool IsConsideredSafe(Stream fileStream);
}

View File

@@ -5,6 +5,7 @@ using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models.Editors;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
@@ -18,6 +19,7 @@ namespace Umbraco.Cms.Core.PropertyEditors;
internal class FileUploadPropertyValueEditor : DataValueEditor
{
private readonly MediaFileManager _mediaFileManager;
private readonly IFileStreamSecurityValidator _fileStreamSecurityValidator;
private ContentSettings _contentSettings;
public FileUploadPropertyValueEditor(
@@ -27,10 +29,12 @@ internal class FileUploadPropertyValueEditor : DataValueEditor
IShortStringHelper shortStringHelper,
IOptionsMonitor<ContentSettings> contentSettings,
IJsonSerializer jsonSerializer,
IIOHelper ioHelper)
IIOHelper ioHelper,
IFileStreamSecurityValidator fileStreamSecurityValidator)
: base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute)
{
_mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager));
_fileStreamSecurityValidator = fileStreamSecurityValidator;
_contentSettings = contentSettings.CurrentValue ?? throw new ArgumentNullException(nameof(contentSettings));
contentSettings.OnChange(x => _contentSettings = x);
}
@@ -147,6 +151,11 @@ internal class FileUploadPropertyValueEditor : DataValueEditor
using (FileStream filestream = File.OpenRead(file.TempFilePath))
{
if (_fileStreamSecurityValidator.IsConsideredSafe(filestream) == false)
{
return null;
}
// TODO: Here it would make sense to do the auto-fill properties stuff but the API doesn't allow us to do that right
// since we'd need to be able to return values for other properties from these methods
_mediaFileManager.FileSystem.AddFile(filepath, filestream, true); // must overwrite!

View File

@@ -10,6 +10,7 @@ using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Editors;
using Umbraco.Cms.Core.PropertyEditors.ValueConverters;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
@@ -24,6 +25,7 @@ namespace Umbraco.Cms.Core.PropertyEditors;
internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core vs web?
{
private readonly IDataTypeService _dataTypeService;
private readonly IFileStreamSecurityValidator _fileStreamSecurityValidator;
private readonly ILogger<ImageCropperPropertyValueEditor> _logger;
private readonly MediaFileManager _mediaFileManager;
private ContentSettings _contentSettings;
@@ -37,13 +39,15 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v
IOptionsMonitor<ContentSettings> contentSettings,
IJsonSerializer jsonSerializer,
IIOHelper ioHelper,
IDataTypeService dataTypeService)
IDataTypeService dataTypeService,
IFileStreamSecurityValidator fileStreamSecurityValidator)
: base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute)
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_mediaFileManager = mediaFileSystem ?? throw new ArgumentNullException(nameof(mediaFileSystem));
_contentSettings = contentSettings.CurrentValue;
_dataTypeService = dataTypeService;
_fileStreamSecurityValidator = fileStreamSecurityValidator;
contentSettings.OnChange(x => _contentSettings = x);
}
@@ -236,6 +240,11 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v
using (FileStream filestream = File.OpenRead(file.TempFilePath))
{
if (_fileStreamSecurityValidator.IsConsideredSafe(filestream) == false)
{
return null;
}
// TODO: Here it would make sense to do the auto-fill properties stuff but the API doesn't allow us to do that right
// since we'd need to be able to return values for other properties from these methods
_mediaFileManager.FileSystem.AddFile(filepath, filestream, true); // must overwrite!

View File

@@ -48,9 +48,43 @@ public class CurrentUserController : UmbracoAuthorizedJsonController
private readonly IShortStringHelper _shortStringHelper;
private readonly IUmbracoMapper _umbracoMapper;
private readonly IUserDataService _userDataService;
private readonly IFileStreamSecurityValidator? _fileStreamSecurityValidator; // make non nullable in v14
private readonly IUserService _userService;
[ActivatorUtilitiesConstructor]
public CurrentUserController(
MediaFileManager mediaFileManager,
IOptionsSnapshot<ContentSettings> contentSettings,
IHostingEnvironment hostingEnvironment,
IImageUrlGenerator imageUrlGenerator,
IBackOfficeSecurityAccessor backofficeSecurityAccessor,
IUserService userService,
IUmbracoMapper umbracoMapper,
IBackOfficeUserManager backOfficeUserManager,
ILocalizedTextService localizedTextService,
AppCaches appCaches,
IShortStringHelper shortStringHelper,
IPasswordChanger<BackOfficeIdentityUser> passwordChanger,
IUserDataService userDataService,
IFileStreamSecurityValidator fileStreamSecurityValidator)
{
_mediaFileManager = mediaFileManager;
_contentSettings = contentSettings.Value;
_hostingEnvironment = hostingEnvironment;
_imageUrlGenerator = imageUrlGenerator;
_backofficeSecurityAccessor = backofficeSecurityAccessor;
_userService = userService;
_umbracoMapper = umbracoMapper;
_backOfficeUserManager = backOfficeUserManager;
_localizedTextService = localizedTextService;
_appCaches = appCaches;
_shortStringHelper = shortStringHelper;
_passwordChanger = passwordChanger;
_userDataService = userDataService;
_fileStreamSecurityValidator = fileStreamSecurityValidator;
}
[Obsolete("Use constructor overload that has fileStreamSecurityValidator, scheduled for removal in v14")]
public CurrentUserController(
MediaFileManager mediaFileManager,
IOptionsSnapshot<ContentSettings> contentSettings,
@@ -254,6 +288,7 @@ public class CurrentUserController : UmbracoAuthorizedJsonController
_contentSettings,
_hostingEnvironment,
_imageUrlGenerator,
_fileStreamSecurityValidator,
_backofficeSecurityAccessor.BackOfficeSecurity?.GetUserId().Result ?? 0);
}

View File

@@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Infrastructure;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;
@@ -51,6 +52,7 @@ public class MediaController : ContentControllerBase
{
private static readonly Semaphore _postAddFileSemaphore = new(1, 1);
private readonly AppCaches _appCaches;
private readonly IFileStreamSecurityValidator? _fileStreamSecurityValidator; // make non nullable in v14
private readonly IAuthorizationService _authorizationService;
private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor;
private readonly ContentSettings _contentSettings;
@@ -67,6 +69,58 @@ public class MediaController : ContentControllerBase
private readonly ISqlContext _sqlContext;
private readonly IUmbracoMapper _umbracoMapper;
[ActivatorUtilitiesConstructor]
public MediaController(
ICultureDictionary cultureDictionary,
ILoggerFactory loggerFactory,
IShortStringHelper shortStringHelper,
IEventMessagesFactory eventMessages,
ILocalizedTextService localizedTextService,
IOptionsSnapshot<ContentSettings> contentSettings,
IMediaTypeService mediaTypeService,
IMediaService mediaService,
IEntityService entityService,
IBackOfficeSecurityAccessor backofficeSecurityAccessor,
IUmbracoMapper umbracoMapper,
IDataTypeService dataTypeService,
ISqlContext sqlContext,
IContentTypeBaseServiceProvider contentTypeBaseServiceProvider,
IRelationService relationService,
PropertyEditorCollection propertyEditors,
MediaFileManager mediaFileManager,
MediaUrlGeneratorCollection mediaUrlGenerators,
IHostingEnvironment hostingEnvironment,
IImageUrlGenerator imageUrlGenerator,
IJsonSerializer serializer,
IAuthorizationService authorizationService,
AppCaches appCaches,
IFileStreamSecurityValidator streamSecurityValidator)
: base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, serializer)
{
_shortStringHelper = shortStringHelper;
_contentSettings = contentSettings.Value;
_mediaTypeService = mediaTypeService;
_mediaService = mediaService;
_entityService = entityService;
_backofficeSecurityAccessor = backofficeSecurityAccessor;
_umbracoMapper = umbracoMapper;
_dataTypeService = dataTypeService;
_localizedTextService = localizedTextService;
_sqlContext = sqlContext;
_contentTypeBaseServiceProvider = contentTypeBaseServiceProvider;
_relationService = relationService;
_propertyEditors = propertyEditors;
_mediaFileManager = mediaFileManager;
_mediaUrlGenerators = mediaUrlGenerators;
_hostingEnvironment = hostingEnvironment;
_logger = loggerFactory.CreateLogger<MediaController>();
_imageUrlGenerator = imageUrlGenerator;
_authorizationService = authorizationService;
_appCaches = appCaches;
_fileStreamSecurityValidator = streamSecurityValidator;
}
[Obsolete("Use constructor overload that has fileStreamSecurityValidator, scheduled for removal in v14")]
public MediaController(
ICultureDictionary cultureDictionary,
ILoggerFactory loggerFactory,
@@ -731,6 +785,17 @@ public class MediaController : ContentControllerBase
continue;
}
using var stream = new MemoryStream();
await formFile.CopyToAsync(stream);
if (_fileStreamSecurityValidator != null && _fileStreamSecurityValidator.IsConsideredSafe(stream) == false)
{
tempFiles.Notifications.Add(new BackOfficeNotification(
_localizedTextService.Localize("speechBubbles", "operationFailedHeader"),
_localizedTextService.Localize("media", "fileSecurityValidationFailure"),
NotificationStyle.Warning));
continue;
}
if (string.IsNullOrEmpty(mediaTypeAlias))
{
mediaTypeAlias = Constants.Conventions.MediaTypes.File;
@@ -808,11 +873,8 @@ public class MediaController : ContentControllerBase
IMedia createdMediaItem = _mediaService.CreateMedia(mediaItemName, parentId.Value, mediaTypeAlias,
_backofficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Id ?? -1);
await using (Stream stream = formFile.OpenReadStream())
{
createdMediaItem.SetValue(_mediaFileManager, _mediaUrlGenerators, _shortStringHelper,
_contentTypeBaseServiceProvider, Constants.Conventions.Media.File, fileName, stream);
}
Attempt<OperationResult?> saveResult = _mediaService.Save(createdMediaItem,
_backofficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Id ?? -1);

View File

@@ -55,6 +55,7 @@ public class UsersController : BackOfficeNotificationsController
private readonly GlobalSettings _globalSettings;
private readonly IHostingEnvironment _hostingEnvironment;
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly IFileStreamSecurityValidator? _fileStreamSecurityValidator; // make non nullable in v14
private readonly IImageUrlGenerator _imageUrlGenerator;
private readonly LinkGenerator _linkGenerator;
private readonly ILocalizedTextService _localizedTextService;
@@ -72,6 +73,58 @@ public class UsersController : BackOfficeNotificationsController
private readonly WebRoutingSettings _webRoutingSettings;
[ActivatorUtilitiesConstructor]
public UsersController(
MediaFileManager mediaFileManager,
IOptionsSnapshot<ContentSettings> contentSettings,
IHostingEnvironment hostingEnvironment,
ISqlContext sqlContext,
IImageUrlGenerator imageUrlGenerator,
IOptionsSnapshot<SecuritySettings> securitySettings,
IEmailSender emailSender,
IBackOfficeSecurityAccessor backofficeSecurityAccessor,
AppCaches appCaches,
IShortStringHelper shortStringHelper,
IUserService userService,
ILocalizedTextService localizedTextService,
IUmbracoMapper umbracoMapper,
IOptionsSnapshot<GlobalSettings> globalSettings,
IBackOfficeUserManager backOfficeUserManager,
ILoggerFactory loggerFactory,
LinkGenerator linkGenerator,
IBackOfficeExternalLoginProviders externalLogins,
UserEditorAuthorizationHelper userEditorAuthorizationHelper,
IPasswordChanger<BackOfficeIdentityUser> passwordChanger,
IHttpContextAccessor httpContextAccessor,
IOptions<WebRoutingSettings> webRoutingSettings,
IFileStreamSecurityValidator fileStreamSecurityValidator)
{
_mediaFileManager = mediaFileManager;
_contentSettings = contentSettings.Value;
_hostingEnvironment = hostingEnvironment;
_sqlContext = sqlContext;
_imageUrlGenerator = imageUrlGenerator;
_securitySettings = securitySettings.Value;
_emailSender = emailSender;
_backofficeSecurityAccessor = backofficeSecurityAccessor;
_appCaches = appCaches;
_shortStringHelper = shortStringHelper;
_userService = userService;
_localizedTextService = localizedTextService;
_umbracoMapper = umbracoMapper;
_globalSettings = globalSettings.Value;
_userManager = backOfficeUserManager;
_loggerFactory = loggerFactory;
_linkGenerator = linkGenerator;
_externalLogins = externalLogins;
_userEditorAuthorizationHelper = userEditorAuthorizationHelper;
_passwordChanger = passwordChanger;
_logger = _loggerFactory.CreateLogger<UsersController>();
_httpContextAccessor = httpContextAccessor;
_fileStreamSecurityValidator = fileStreamSecurityValidator;
_webRoutingSettings = webRoutingSettings.Value;
}
[Obsolete("Use constructor overload that has fileStreamSecurityValidator, scheduled for removal in v14")]
public UsersController(
MediaFileManager mediaFileManager,
IOptionsSnapshot<ContentSettings> contentSettings,
@@ -139,13 +192,15 @@ public class UsersController : BackOfficeNotificationsController
[AppendUserModifiedHeader("id")]
[Authorize(Policy = AuthorizationPolicies.AdminUserEditsRequireAdmin)]
public IActionResult PostSetAvatar(int id, IList<IFormFile> file) => PostSetAvatarInternal(file, _userService,
public IActionResult PostSetAvatar(int id, IList<IFormFile> file)
=> PostSetAvatarInternal(file, _userService,
_appCaches.RuntimeCache, _mediaFileManager, _shortStringHelper, _contentSettings, _hostingEnvironment,
_imageUrlGenerator, id);
_imageUrlGenerator,_fileStreamSecurityValidator, id);
internal static IActionResult PostSetAvatarInternal(IList<IFormFile> files, IUserService userService,
IAppCache cache, MediaFileManager mediaFileManager, IShortStringHelper shortStringHelper,
ContentSettings contentSettings, IHostingEnvironment hostingEnvironment, IImageUrlGenerator imageUrlGenerator,
IFileStreamSecurityValidator? fileStreamSecurityValidator,
int id)
{
if (files is null)
@@ -187,9 +242,14 @@ public class UsersController : BackOfficeNotificationsController
//generate a path of known data, we don't want this path to be guessable
user.Avatar = "UserAvatars/" + (user.Id + safeFileName).GenerateHash<SHA1>() + "." + ext;
using (Stream fs = file.OpenReadStream())
//todo implement Filestreamsecurity
using (var ms = new MemoryStream())
{
mediaFileManager.FileSystem.AddFile(user.Avatar, fs, true);
file.CopyTo(ms);
if(fileStreamSecurityValidator != null && fileStreamSecurityValidator.IsConsideredSafe(ms) == false)
return new ValidationErrorResult("One or more file security analyzers deemed the contents of the file to be unsafe");
mediaFileManager.FileSystem.AddFile(user.Avatar, ms, true);
}
userService.Save(user);

View File

@@ -68,10 +68,10 @@ public abstract class ContentTypeModelValidatorBase<TModel, TProperty> : EditorV
private ValidationResult? ValidateProperty(PropertyTypeBasic property, int groupIndex, int propertyIndex)
{
// don't let them match any properties or methods in IPublishedContent
// don't let them match any properties or methods in IPublishedContent (including those defined in any base interfaces like IPublishedElement)
// TODO: There are probably more!
var reservedProperties = typeof(IPublishedContent).GetProperties().Select(x => x.Name).ToArray();
var reservedMethods = typeof(IPublishedContent).GetMethods().Select(x => x.Name).ToArray();
var reservedProperties = typeof(IPublishedContent).GetPublicProperties().Select(x => x.Name).ToArray();
var reservedMethods = typeof(IPublishedContent).GetPublicMethods().Select(x => x.Name).ToArray();
var alias = property.Alias;

View File

@@ -0,0 +1,123 @@
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.Security;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Security;
public class FileStreamSecurityValidatorTests
{
[Test]
public void IsConsideredSafe_True_NoAnalyzersPresent()
{
// Arrange
var sut = new FileStreamSecurityValidator(Enumerable.Empty<IFileStreamSecurityAnalyzer>());
using var memoryStream = new MemoryStream();
using var streamWriter = new StreamWriter(memoryStream);
streamWriter.Write("TestContent");
streamWriter.Flush();
memoryStream.Seek(0, SeekOrigin.Begin);
// Act
var validationResult = sut.IsConsideredSafe(memoryStream);
// Assert
Assert.IsTrue(validationResult);
}
[Test]
public void IsConsideredSafe_True_NoAnalyzerMatchesType()
{
// Arrange
var analyzerOne = new Mock<IFileStreamSecurityAnalyzer>();
analyzerOne.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(false);
var analyzerTwo = new Mock<IFileStreamSecurityAnalyzer>();
analyzerTwo.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(false);
var sut = new FileStreamSecurityValidator(new List<IFileStreamSecurityAnalyzer>{analyzerOne.Object,analyzerTwo.Object});
using var memoryStream = new MemoryStream();
using var streamWriter = new StreamWriter(memoryStream);
streamWriter.Write("TestContent");
streamWriter.Flush();
memoryStream.Seek(0, SeekOrigin.Begin);
// Act
var validationResult = sut.IsConsideredSafe(memoryStream);
// Assert
Assert.IsTrue(validationResult);
}
[Test]
public void IsConsideredSafe_True_AllMatchingAnalyzersReturnTrue()
{
// Arrange
var matchingAnalyzerOne = new Mock<IFileStreamSecurityAnalyzer>();
matchingAnalyzerOne.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(true);
matchingAnalyzerOne.Setup(analyzer => analyzer.IsConsideredSafe(It.IsAny<Stream>()))
.Returns(true);
var matchingAnalyzerTwo = new Mock<IFileStreamSecurityAnalyzer>();
matchingAnalyzerTwo.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(true);
matchingAnalyzerTwo.Setup(analyzer => analyzer.IsConsideredSafe(It.IsAny<Stream>()))
.Returns(true);
var unmatchedAnalyzer = new Mock<IFileStreamSecurityAnalyzer>();
unmatchedAnalyzer.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(false);
var sut = new FileStreamSecurityValidator(new List<IFileStreamSecurityAnalyzer>{matchingAnalyzerOne.Object,matchingAnalyzerTwo.Object});
using var memoryStream = new MemoryStream();
using var streamWriter = new StreamWriter(memoryStream);
streamWriter.Write("TestContent");
streamWriter.Flush();
memoryStream.Seek(0, SeekOrigin.Begin);
// Act
var validationResult = sut.IsConsideredSafe(memoryStream);
// Assert
Assert.IsTrue(validationResult);
}
[Test]
public void IsConsideredSafe_False_AnyMatchingAnalyzersReturnFalse()
{
// Arrange
var saveMatchingAnalyzer = new Mock<IFileStreamSecurityAnalyzer>();
saveMatchingAnalyzer.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(true);
saveMatchingAnalyzer.Setup(analyzer => analyzer.IsConsideredSafe(It.IsAny<Stream>()))
.Returns(true);
var unsafeMatchingAnalyzer = new Mock<IFileStreamSecurityAnalyzer>();
unsafeMatchingAnalyzer.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(true);
unsafeMatchingAnalyzer.Setup(analyzer => analyzer.IsConsideredSafe(It.IsAny<Stream>()))
.Returns(false);
var unmatchedAnalyzer = new Mock<IFileStreamSecurityAnalyzer>();
unmatchedAnalyzer.Setup(analyzer => analyzer.ShouldHandle(It.IsAny<Stream>()))
.Returns(false);
var sut = new FileStreamSecurityValidator(new List<IFileStreamSecurityAnalyzer>{saveMatchingAnalyzer.Object,unsafeMatchingAnalyzer.Object});
using var memoryStream = new MemoryStream();
using var streamWriter = new StreamWriter(memoryStream);
streamWriter.Write("TestContent");
streamWriter.Flush();
memoryStream.Seek(0, SeekOrigin.Begin);
// Act
var validationResult = sut.IsConsideredSafe(memoryStream);
// Assert
Assert.IsFalse(validationResult);
}
}