-
Notifications
You must be signed in to change notification settings - Fork 2.9k
v12: Add HMAC image processing protection #14181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
ff153cd
Update to ImageSharp 2.1.0 and ImageSharp.Web 2.0.0-alpha.0.23
ronaldbarendse e8d82af
Rename CachedNameLength to CacheHashLength and add CacheFolderDepth s…
ronaldbarendse 5fa02e9
Replace PhysicalFileSystemProvider with WebRootImageProvider
ronaldbarendse da34c09
Support EXIF-orientation in image dimention extractor
ronaldbarendse 91fbdb2
Remove virtual methods on FileProviderImageProvider
ronaldbarendse 827ae60
Simplify FileInfoImageResolver
ronaldbarendse 8efb526
Update to SixLabors.ImageSharp.Web 2.0.0-alpha.0.25 and remove custom…
ronaldbarendse d35dfc6
Make CropWebProcessor EXIF orientation-aware
ronaldbarendse ba491ec
Improve width/height sanitization
ronaldbarendse 5b973a9
Also use 'v' as cache buster value
ronaldbarendse 2cbef09
Add WebP to supported image file types
ronaldbarendse e7f74dc
Update to SixLabors.ImageSharp.Web 2.0.0-alpha.0.27 and fix test
ronaldbarendse 70e2437
Fix rounding error and add test cases
ronaldbarendse 67b8dbc
Update to newest and stable releases
ronaldbarendse 5b758db
Merge branch 'v10/dev' into v10/feature/imagesharp2
ronaldbarendse 798a3bd
Merge branch 'origin/v10/dev' into v10/feature/imagesharp2
ronaldbarendse cac5402
Move ImageSharpImageUrlGenerator to Umbraco.Web.Common
ronaldbarendse 3cae534
Use IConfigureOptions to configure ImageSharp options
ronaldbarendse 10eb181
Implement IEquatable on ImageUrlGenerationOptions classes
ronaldbarendse 2de5520
Fix empty/null values in image URL generation and corresponding tests
ronaldbarendse 5898b0c
Use IsSupportedImageFormat extension method
ronaldbarendse 5cb9a9b
Remove unneeded reflection
ronaldbarendse bd4bb54
Add HMACSecretKey setting and add token when generating image URLs
ronaldbarendse 3dd078a
Ensure backoffice image URLs are generated by the server (and include…
ronaldbarendse 8bd54f2
Abstract HMAC generation to IImageUrlTokenGenerator
ronaldbarendse 8ee6938
Merge branch 'v10/dev' into v10/feature/imagesharp2-hmacsecretkey
ronaldbarendse e16940e
Change cache buster value to 'v' and use hexadecimal timestamp
ronaldbarendse 845cadc
Update comments
ronaldbarendse c17c58c
Merge branch 'v10/dev' into v10/feature/imagesharp2-hmacsecretkey
ronaldbarendse 9e7141f
Fix backoffice thumbnail URL generation
ronaldbarendse c9401c9
Update grid media thumbnail URL generation
ronaldbarendse 19c01cf
Merge branch 'v10/dev' into v10/feature/imagesharp2-hmacsecretkey
ronaldbarendse 11b16c9
Merge branch 'v10/dev' into v10/feature/imagesharp2-hmacsecretkey
ronaldbarendse d9879ff
Remove breaking changes
ronaldbarendse 577f866
Strip unknown commands from image URL token
ronaldbarendse c627a90
Remove HMAC whitelisting possibility (not supported by ImageSharp)
ronaldbarendse e19b2d2
Update to SixLabors.ImageSharp 2.1.3
ronaldbarendse 1748b78
Add comment to internal constructor
ronaldbarendse 69ab5af
Fix to support absolute image URLs
ronaldbarendse 5274346
Merge branch 'v10/dev' into v10/feature/imagesharp2-hmacsecretkey
ronaldbarendse dd184d0
Update to SixLabors.ImageSharp.Web 2.0.3-alpha.0.3
ronaldbarendse 40f694d
Remove IImageUrlTokenGenerator and use ImageSharpRequestAuthorization…
ronaldbarendse 5f0f351
Move NuGet feed to config file
ronaldbarendse 41712f3
Merge branch 'v10/dev' into v10/feature/imagesharp2-hmacsecretkey
ronaldbarendse b4296a5
Update to ImageSharp v3
ronaldbarendse dcf0d19
Merge branch 'v10/feature/imagesharp2-hmacsecretkey' into v12/feature…
ronaldbarendse b09a96c
Merge branch 'v12/dev' into v12/feature/imagesharp3-hmacsecretkey
ronaldbarendse File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| using System.Globalization; | ||
| using Microsoft.AspNetCore.WebUtilities; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Primitives; | ||
| using SixLabors.ImageSharp.Web.Processors; | ||
| using Umbraco.Cms.Core.DependencyInjection; | ||
| using Umbraco.Cms.Core.Media; | ||
| using Umbraco.Cms.Core.Models; | ||
| using Umbraco.Cms.Imaging.ImageSharp.ImageProcessors; | ||
|
|
@@ -10,31 +12,47 @@ | |
| namespace Umbraco.Cms.Imaging.ImageSharp.Media; | ||
|
|
||
| /// <summary> | ||
| /// Exposes a method that generates an image URL based on the specified options that can be processed by ImageSharp. | ||
| /// Exposes a method that generates an image URL based on the specified options that can be processed by ImageSharp. | ||
| /// </summary> | ||
| /// <seealso cref="IImageUrlGenerator" /> | ||
| public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator | ||
| { | ||
| /// <inheritdoc /> | ||
| public IEnumerable<string> SupportedImageFileTypes { get; } | ||
| private readonly RequestAuthorizationUtilities? _requestAuthorizationUtilities; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="ImageSharpImageUrlGenerator" /> class. | ||
| /// </summary> | ||
| /// <param name="configuration">The ImageSharp configuration.</param> | ||
| /// <param name="requestAuthorizationUtilities">Contains helpers that allow authorization of image requests.</param> | ||
| public ImageSharpImageUrlGenerator(Configuration configuration, RequestAuthorizationUtilities? requestAuthorizationUtilities) | ||
| : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray(), requestAuthorizationUtilities) | ||
| { } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="ImageSharpImageUrlGenerator" /> class. | ||
| /// Initializes a new instance of the <see cref="ImageSharpImageUrlGenerator" /> class. | ||
| /// </summary> | ||
| /// <param name="configuration">The ImageSharp configuration.</param> | ||
| [Obsolete("Use ctor with all params - This will be removed in Umbraco 13.")] | ||
| public ImageSharpImageUrlGenerator(Configuration configuration) | ||
| : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray()) | ||
| : this(configuration, StaticServiceProvider.Instance.GetService<RequestAuthorizationUtilities>()) | ||
| { } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="ImageSharpImageUrlGenerator" /> class. | ||
| /// Initializes a new instance of the <see cref="ImageSharpImageUrlGenerator" /> class. | ||
| /// </summary> | ||
| /// <param name="supportedImageFileTypes">The supported image file types/extensions.</param> | ||
| /// <param name="requestAuthorizationUtilities">Contains helpers that allow authorization of image requests.</param> | ||
| /// <remarks> | ||
| /// This constructor is only used for testing. | ||
| /// This constructor is only used for testing. | ||
| /// </remarks> | ||
| internal ImageSharpImageUrlGenerator(IEnumerable<string> supportedImageFileTypes) => | ||
| internal ImageSharpImageUrlGenerator(IEnumerable<string> supportedImageFileTypes, RequestAuthorizationUtilities? requestAuthorizationUtilities = null) | ||
| { | ||
| SupportedImageFileTypes = supportedImageFileTypes; | ||
| _requestAuthorizationUtilities = requestAuthorizationUtilities; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public IEnumerable<string> SupportedImageFileTypes { get; } | ||
|
|
||
| /// <inheritdoc /> | ||
| public string? GetImageUrl(ImageUrlGenerationOptions? options) | ||
|
|
@@ -47,57 +65,63 @@ internal ImageSharpImageUrlGenerator(IEnumerable<string> supportedImageFileTypes | |
| var queryString = new Dictionary<string, string?>(); | ||
| Dictionary<string, StringValues> furtherOptions = QueryHelpers.ParseQuery(options.FurtherOptions); | ||
|
|
||
| if (options.Crop is not null) | ||
| if (options.Crop is CropCoordinates crop) | ||
| { | ||
| CropCoordinates? crop = options.Crop; | ||
| queryString.Add( | ||
| CropWebProcessor.Coordinates, | ||
| FormattableString.Invariant($"{crop.Left},{crop.Top},{crop.Right},{crop.Bottom}")); | ||
| queryString.Add(CropWebProcessor.Coordinates, FormattableString.Invariant($"{crop.Left},{crop.Top},{crop.Right},{crop.Bottom}")); | ||
| } | ||
|
|
||
| if (options.FocalPoint is not null) | ||
| if (options.FocalPoint is FocalPointPosition focalPoint) | ||
| { | ||
| queryString.Add(ResizeWebProcessor.Xy, FormattableString.Invariant($"{options.FocalPoint.Left},{options.FocalPoint.Top}")); | ||
| queryString.Add(ResizeWebProcessor.Xy, FormattableString.Invariant($"{focalPoint.Left},{focalPoint.Top}")); | ||
| } | ||
|
|
||
| if (options.ImageCropMode is not null) | ||
| if (options.ImageCropMode is ImageCropMode imageCropMode) | ||
| { | ||
| queryString.Add(ResizeWebProcessor.Mode, options.ImageCropMode.ToString()?.ToLowerInvariant()); | ||
| queryString.Add(ResizeWebProcessor.Mode, imageCropMode.ToString().ToLowerInvariant()); | ||
| } | ||
|
|
||
| if (options.ImageCropAnchor is not null) | ||
| if (options.ImageCropAnchor is ImageCropAnchor imageCropAnchor) | ||
| { | ||
| queryString.Add(ResizeWebProcessor.Anchor, options.ImageCropAnchor.ToString()?.ToLowerInvariant()); | ||
| queryString.Add(ResizeWebProcessor.Anchor, imageCropAnchor.ToString().ToLowerInvariant()); | ||
| } | ||
|
|
||
| if (options.Width is not null) | ||
| if (options.Width is int width) | ||
| { | ||
| queryString.Add(ResizeWebProcessor.Width, options.Width?.ToString(CultureInfo.InvariantCulture)); | ||
| queryString.Add(ResizeWebProcessor.Width, width.ToString(CultureInfo.InvariantCulture)); | ||
| } | ||
|
|
||
| if (options.Height is not null) | ||
| if (options.Height is int height) | ||
| { | ||
| queryString.Add(ResizeWebProcessor.Height, options.Height?.ToString(CultureInfo.InvariantCulture)); | ||
| queryString.Add(ResizeWebProcessor.Height, height.ToString(CultureInfo.InvariantCulture)); | ||
| } | ||
|
|
||
| if (furtherOptions.Remove(FormatWebProcessor.Format, out StringValues format)) | ||
| { | ||
| queryString.Add(FormatWebProcessor.Format, format[0]); | ||
| queryString.Add(FormatWebProcessor.Format, format.ToString()); | ||
| } | ||
|
|
||
| if (options.Quality is not null) | ||
| if (options.Quality is int quality) | ||
| { | ||
| queryString.Add(QualityWebProcessor.Quality, options.Quality?.ToString(CultureInfo.InvariantCulture)); | ||
| queryString.Add(QualityWebProcessor.Quality, quality.ToString(CultureInfo.InvariantCulture)); | ||
| } | ||
|
|
||
| foreach (KeyValuePair<string, StringValues> kvp in furtherOptions) | ||
| { | ||
| queryString.Add(kvp.Key, kvp.Value); | ||
| } | ||
|
|
||
| if (options.CacheBusterValue is not null && !string.IsNullOrWhiteSpace(options.CacheBusterValue)) | ||
| if (options.CacheBusterValue is string cacheBusterValue && !string.IsNullOrEmpty(cacheBusterValue)) | ||
| { | ||
| queryString.Add("v", cacheBusterValue); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The cache buster value is now using the |
||
| } | ||
|
|
||
| if (_requestAuthorizationUtilities is not null) | ||
| { | ||
| queryString.Add("rnd", options.CacheBusterValue); | ||
| var uri = QueryHelpers.AddQueryString(options.ImageUrl, queryString); | ||
| if (_requestAuthorizationUtilities.ComputeHMAC(uri, CommandHandling.Sanitize) is string token && !string.IsNullOrEmpty(token)) | ||
| { | ||
| queryString.Add(RequestAuthorizationUtilities.TokenCommand, token); | ||
| } | ||
| } | ||
|
|
||
| return QueryHelpers.AddQueryString(options.ImageUrl, queryString); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can safely skip validating the maximum width/height when using HMAC authentication (the default callback in V2 also removed the hardcoded width/height check).