search

vnbaaij / ImageProcessor.Web.Episerver

Integrate ImageProcessor into Episerver and use it in your views with a fluent API. For Editor integration do not use ImageProcessor.Web.Episerver.UI but instead use ImageProcessor.Web.Episerver.UI.Blocks. For more information see
http://world.episerver.com/blogs/vincent-baaij/dates/2017/10/episerver-and-imageprocessor-more-choice-for-developers-and-designers/
MIT License
20 stars 9 forks source link

Possible DoS: adding dummy parameters to query leads to creation of new image in cache #52

Closed lanorkin closed 3 years ago

lanorkin commented 3 years ago

Description

Cache key is based on full URL & query string, so adding any dummy parameters to query leads to new image generation.

That can be used for DoS attacks, which will utilize CPU / Memory for unneeded image resizes, and will easily fill file / blob cache with duplicates of resized images.

Related issue: file is being put into cache (with original size), even if it was not processed due to RestrictTo limitations in config.

Steps to reproduce

  1. Open any file with resize-based URL, like ?width=600&mode=min.
  2. Ensure file was created in cache (file or azure blobs)
  3. Add dummy parameter to query, like ?width=600&mode=min&dummy=1.

Expected result: No new file in cache

Actual result: New file in cache generated

Possible cause for the issue

FileBlobCache and AzureBlobCache both use CreateCachedFileNameAsync to generate cache key:

https://github.com/vnbaaij/ImageProcessor.Web.Episerver/blob/2b1aec9aa33562654f4ad48f7ff6880e12d17664/src/ImageProcessor.Web.Episerver/FileBlobCache.cs#L119

https://github.com/vnbaaij/ImageProcessor.Web.Episerver/blob/2b1aec9aa33562654f4ad48f7ff6880e12d17664/src/ImageProcessor.Web.Episerver.Azure/AzureBlobCache.cs#L118

which inside uses CachedImageHelper.GetCachedImageFileName

https://github.com/JimBobSquarePants/ImageProcessor/blob/a94fb903e49f6fa5f47c6ea58d27af55ce8ff89c/src/ImageProcessor.Web/Caching/ImageCacheBase.cs#L155

which just hashes full path:

https://github.com/JimBobSquarePants/ImageProcessor/blob/a94fb903e49f6fa5f47c6ea58d27af55ce8ff89c/src/ImageProcessor.Web/Caching/CachedImageHelper.cs#L65

This leads to creating new cache key for every new query.

Maybe it will be useful to parse query and hash only meaningful query parameters - and not sure if it is better to fix here, or ask @JimBobSquarePants for help in ImageCacheBase

JimBobSquarePants commented 3 years ago

Please don’t tag me like this

vnbaaij commented 3 years ago

Hi, will not be spending time fixing this. It is true that this could be used to attack a server but it would take a fair amount of effort to do so. The library just works this way (using querystring params). If this is an unacceptable risk for you, then don't use it. Or contribute a fix...