Fix memory DoS and extension bypass in attribute file uploads - #789
Merged
Conversation
Contact/Product/Checkout attribute upload endpoints buffered the entire file into memory before checking the size limit, and treated an empty ValidationFileAllowedExtensions as "any extension allowed". An unauthenticated user could exhaust server memory with repeated large uploads, or store arbitrary file content. Check IFormFile.Length against a hard cap (min of the attribute's configured limit and a new 10 MB ceiling) before reading the body, and fall back to the existing safe default extension allow-list (already used by PictureController) when the attribute has none configured. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ASP.NET Core already provides a declarative way to cap request body size per action; use it instead of hand-rolled Math.Min/byte-comparison logic in the controller. The framework now rejects an oversized request before the multipart body is parsed at all, which is a stronger guarantee than checking IFormFile.Length in application code. The per-attribute ValidationFileMaximumSize check (via file.Length, before reading the body) is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tribute Grand.Web already exposes AppConfig.MaxRequestBodySize, wired to both Kestrel's Limits.MaxRequestBodySize and FormOptions.MultipartBodyLengthLimit (ConfigurationExtensions.ConfigureApplicationSettings). It was left null, falling back to Kestrel's 30MB default. Set it to 10MB in Grand.Web's appsettings.json instead of introducing a custom [RequestSizeLimit] attribute/constant per action - reuses the mechanism the app already has for this, applies to every storefront endpoint, and stays admin-configurable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves #issueNumber
Type: bugfix
Issue
ContactController.UploadFileContactAttribute,ProductController.UploadFileProductAttribute, andShoppingCartController.UploadFileCheckoutAttribute(all reachable from the storefront, two without authentication required) buffered the entire uploaded file into abyte[]viaIFormFile.GetDownloadBits()before checkingValidationFileMaximumSize.Application.MaxRequestBodySizeinappsettings.jsonwas leftnull, so only Kestrel's default 30 MB request-body limit applied. An unauthenticated user could repeatedly upload files up to that limit, each fully allocated in memory before being rejected - a cheap memory-exhaustion DoS.Separately, when an attribute had no
ValidationFileAllowedExtensionsconfigured, the extension check was skipped entirely, so any file extension was accepted and stored.To reproduce (pre-fix): configure a contact/checkout/product attribute of type File Upload with no allowed-extensions list, then POST a large file (e.g. 20 MB) to
UploadFileContactAttributeas an anonymous user - the full body is read into memory regardless of any configured size limit, and any extension is accepted.Solution
Application.MaxRequestBodySizeto 10 MB inGrand.Web/App_Data/appsettings.json. This setting already exists and is wired to bothKestrel.Limits.MaxRequestBodySizeandFormOptions.MultipartBodyLengthLimit(ConfigurationExtensions.ConfigureApplicationSettings) - ASP.NET Core now rejects an oversized request for every storefront endpoint before the body is parsed, without any new code. Admin-configurable, as before.ValidationFileMaximumSizecheck, comparing againstIFormFile.Length(size reported by the multipart headers) beforeGetDownloadBits()is called, so a request within the 10 MB ceiling but over a smaller attribute-configured limit is still rejected without being buffered.ValidationFileAllowedExtensions.Split(...)check with the existingFileExtensions.GetAllowedMediaFileTypes(...)helper (already used byPictureController), which falls back to a safe image-extension allow-list when the attribute has none configured, instead of allowing everything.Breaking changes
Any Grand.Web storefront request body over 10 MB (not just these three endpoints) is now rejected by Kestrel; previously up to 30 MB was allowed. Adjust
Application.MaxRequestBodySizeinappsettings.jsonif a store needs a higher storefront-wide limit. Attributes with no configured allowed-extensions list now restrict uploads to.gif/.jpg/.jpeg/.png/.bmp/.webpinstead of accepting any extension - a deliberate secure-by-default change to previously unrestricted behavior.Testing
.exefile to the corresponding upload endpoint - it is now rejected (ValidationFileAllowedmessage) instead of being accepted.