Repository navigation
Conversation
kian0417
marked this pull request as draft
October 8, 2026 01:34
kian0417
marked this pull request as ready for review
October 8, 2026 03:43
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
The current default branch still renders editable attachment values as HTML in the backend attachment table and the frontend user attachment table. A restricted backend account with only attachment view/edit rules (
2,7,23,26) can edit an attachment owned by another user, setuser_id, and persist an attribute-breaking URL. The value then executes when a higher-privileged administrator opens the attachment manager or the target user opens their attachment page. This is the rendering path associated with the already assigned CVE-2024-7453.Fix
frontend/user.js, which was a separate reachable sink.htmlspecialcharsfilename entities before re-escaping, so existing names such asaudit&A.pngdisplay asaudit&A.pngwithout becoming HTML.End-to-end validation
I tested the vulnerable commit
7b1c865adb184914f3c2ab92c098173f330468fbagainst this fix atca60455f5e972086d010514007942b85f6e848c6using PHP 7.4.30, MySQL 8.0.42, and Chrome 154. The two deployments used separate databases and sessions. Only the admin login CAPTCHA was disabled in the local fixture; username/password authentication, CSRF token handling, sessions, attachment permissions, and the denied admin-management request remained active.The restricted attachment editor successfully logged in as admin id 2 and edited attachment id 1 through the real HTTP controller. The same account received
code: 0for/auth/admin/index, confirming it was not a super administrator. The exact payload was present in MySQL in both deployments.dataset.faCve=7453, titleFA-XSS-PROOF, one injectedimg[onerror]).img[onerror]; the complete attacker string remained data inside the escapedsrcvalue./assets/img/qrcode.pngat 150x150, kept the correct link, and displayed the legacy stored filenameaudit&A.pngasaudit&A.png.The browser-loaded backend and frontend files matched the committed files by SHA-256.
Checks
node tests/attachment-formatters.test.js: 10/10 pass.node --checkpasses for both changed controller modules and the test file.git diff --check 7b1c865..ca60455: passes.The repository's existing
npm run builddoes not complete in a clean checkout because its optimizer references the absentpublic/assets/js/moment.js. The changed controller modules are loaded dynamically in production mode and were exercised directly by the browser E2E above.Original vulnerability report and discovery credit: Hebing123's report. CVE reference: CVE-2024-7453.