fix(server): security hardening — SECU audit batch 1#660
Merged
Conversation
added 16 commits
March 1, 2026 14:55
The UpdateUser handler checked MaxTTL and MaxFileSize to prevent non-admin users from modifying their own quotas, but did not check MaxUserSize. A non-admin user could increase their own storage quota by sending a modified MaxUserSize value in the update request. Add MaxUserSize to the existing quota check and add a dedicated test.
When requesting a file that exists but belongs to a different upload, the server returned HTTP 500 instead of 404. This allowed an attacker to distinguish between non-existent file IDs (404) and file IDs that exist in other uploads (500), enabling file ID enumeration. Return 404 with the same message as a genuinely missing file.
When a client sent protectedByPassword=true with an empty password, the server silently created an unprotected upload. This gave users a false sense of security — they believed their upload was password- protected when it was not. Now return a 400 error with message 'upload password is empty' when the client explicitly requests password protection but provides no password.
The Logout handler called GetAuthenticator() which panics when the authenticator is nil (authentication disabled). The panic recovery middleware then returned HTTP 500. Add GetAuthenticatorSafe() that returns nil instead of panicking, and use it in the Logout handler to return 200 gracefully.
Extend content-type neutralization to cover SVG, XML, and JavaScript in addition to the existing HTML, Flash, and PDF handling. All dangerous types are now consistently forced to application/octet-stream to prevent XSS via SVG onload handlers, XML rendering, or script execution. Document the neutralization rules in the security guide.
The comments field had no server-side length validation, allowing clients to submit arbitrarily large comments. Add a 10,000 character maximum and return an error when exceeded.
Anchor tags opening in a new tab without rel="noopener" allow the opened page to access window.opener. Add the attribute to both the filename link and the download button in FileRow.vue.
Truncate filenames to 1024 characters in SanitizeFilenameForDisposition as defense-in-depth. The CreateFile path already validates length, but this catches any code path that serves filenames in headers.
…lenames Unicode bidirectional override characters (U+202A-U+202E, U+2066-U+2069) can trick users about file extensions. For example, a filename containing RLO (U+202E) can make 'evil<RLO>fdp.exe' appear as 'evil exe.pdf' in some UIs. Strip all 9 BiDi formatting characters from filenames.
Replace plain == comparisons with crypto/subtle.ConstantTimeCompare for upload token (X-UploadToken header) and API token verification. Prevents theoretical timing attacks on token values.
Wrap /clients/, /changelog/, and webapp root FileServer handlers with NoDirListing to return 404 for directory requests instead of generating listings. Root '/' is allowed through for SPA index.html.
Session and XSRF cookies now get the Secure flag when either EnhancedWebSecurity or SslEnabled is true, ensuring cookies are never sent over plain HTTP when TLS is configured.
Add Strict-Transport-Security middleware (max-age=31536000, 1 year) applied when SslEnabled or EnhancedWebSecurity is true. Tells browsers to always use HTTPS for this domain. Updated security docs.
Replace MD5 with bcrypt(sha256(base64(login:password))) for upload password storage. SHA-256 pre-hash removes bcrypt's 72-byte input limit. - Add common.HashUploadPassword and common.CheckUploadPassword helpers - Validate login and password length (max 128 characters each) - Legacy MD5 hashes are still accepted for old uploads until they expire - Add tests for bcrypt path, legacy MD5 path, and length validation - Document password hashing scheme and limits in docs/guide/security.md
- Document SECU-20: removable uploads are deletable by anyone by design - Document SECU-02: bcrypt(sha256) password hashing with 128-char limits - Document legacy MD5 compatibility for uploads created before 1.4
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
1 participant
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.
What
Batch of security fixes and hardening based on a security audit by @bewiwi. Covers input validation, authentication, content-type safety, transport security, and documentation.
Commits
31f7971fix(server): prevent non-admin MaxUserSize quota bypass4b03a1dfix(server): return 404 for cross-upload file access876850dfix(server): reject empty password when protection requested347de0bfix(server): handle logout gracefully when auth disabled37ea797fix(server): neutralize SVG/XML/JS content types9207060chore: gitignore security review documents063192dfix(server): limit upload comment length to 32KB586dd35fix(webapp): addrel="noopener noreferrer"to download links8c48352fix(server): enforce filename length limit in Content-Disposition79508e1fix(server): strip Unicode BiDi overrides from Content-Disposition69d9578fix(server): constant-time token comparisond3b09f2fix(server): disable directory listing on static handlers89ba3f3fix(server): set Secure cookie flag when TLS enablede29dbc5fix(server): add HSTS header when TLS enablede9e978dfix(server): use bcrypt(sha256) for upload password hashingeee190cdocs(security): document removable upload + password hashingChanges
Authentication & Crypto
bcrypt(sha256(base64(login:password)))with 128-char limitsSslEnabled || EnhancedWebSecurityprotectedByPassword=trueContent Security
application/octet-streamrel="noopener noreferrer"to download linksTransport Security
max-age=31536000) when TLS is enabledInput Validation
Information Disclosure
/clients/,/changelog/, webapp rootDocumentation
Testing
Each commit includes relevant unit tests. All
make lint,make client server,make docs,make vulnpass.Thanks to @bewiwi for the thorough security audit that identified these issues. 🙏