fix: unbounded upload size when scan=false - #514
Closed
cschuerings wants to merge 1 commit into
Closed
Conversation
validateAttachmentSize used MAX_FILE_SIZE (a Function) as a plain value, so `length > MAX_FILE_SIZE` was always false (number vs. Function object), and any upload size was accepted on entities without @Validation.Maximum. Two fixes: 1. Replace the MAX_FILE_SIZE function reference with DEFAULT_MAX_FILE_SIZE (the 400 MB constant) so the comparison is always number vs. number. 2. Don't let MAX_FILE_SIZE()'s -1 (scan=false sentinel) flow into validateAttachmentSize — the upload size limit must apply regardless of whether malware scanning is enabled (README: "The default is 400MB"). MAX_FILE_SIZE() and its -1-when-scan=false behaviour are unchanged; DEFAULT_MAX_FILE_SIZE is exported so callers that need a hard cap can use it directly without re-deriving the magic number.
cschuerings
requested a deployment
to
pr-approval
August 4, 2026 16:28 — with
GitHub Actions
Waiting
Contributor
|
This is intended behavior, if no malware scanner is present then there is no need for a file size limit. |
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.
Problem
validateAttachmentSize(lib/generic-handlers.js) fell back toMAX_FILE_SIZE(a function reference) instead of calling it, solength > MAX_FILE_SIZEwas alwaysfalse— any body size was accepted on entities without@Validation.Maximum.Even when called correctly,
MAX_FILE_SIZE()returns-1whenscan=false, andcreateSizeCheckHandlertreats-1as "no limit". Together, both paths produced an unbounded upload.Fix
MAX_FILE_SIZEin favour of a plainDEFAULT_MAX_FILE_SIZEconstant (400 MB). The constant is always a real byte limit regardless of whether malware scanning is enabled, matching the documented default ("The default is 400MB" in README).sizeInBytesfallbacks updated to useDEFAULT_MAX_FILE_SIZEfor the same reason.Out of scope
Uploads with a valid
@Validation.Maximumannotation were not affected by this bug — the annotation path calledsizeInBytes()correctly.