Skip to content

fix: prevent stored XSS in attachment tables (CVE-2024-7453) - #86

Open
kian0417 wants to merge 2 commits into
fastadminnet:1.6.x-devfrom
kian0417:security/cve-2024-7453
Open

kian0417 wants to merge 2 commits into
fastadminnet:1.6.x-devfrom
kian0417:security/cve-2024-7453

Conversation

@kian0417

@kian0417 kian0417 commented Oct 8, 2026 •

Copy link
Copy Markdown

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, set user_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

  • Escape URL, filename, MIME type, storage, image type, and dimensions at their HTML rendering boundaries.
  • Reject control characters and explicit active schemes while preserving relative URLs, protocol-relative URLs, and HTTP(S) URLs.
  • Apply the same preview formatter to frontend/user.js, which was a separate reachable sink.
  • Decode one layer of legacy PHP htmlspecialchars filename entities before re-escaping, so existing names such as audit&A.png display as audit&A.png without becoming HTML.
  • Add focused tests that load the real AMD modules for both backend and frontend formatters.

End-to-end validation

I tested the vulnerable commit 7b1c865adb184914f3c2ab92c098173f330468fb against this fix at ca60455f5e972086d010514007942b85f6e848c6 using 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: 0 for /auth/admin/index, confirming it was not a super administrator. The exact payload was present in MySQL in both deployments.

  • Baseline backend: the harmless local marker executed (dataset.faCve=7453, title FA-XSS-PROOF, one injected img[onerror]).
  • Baseline frontend: the same marker executed for the target user.
  • Patched backend, frontend, and attachment selector: marker absent and zero injected img[onerror]; the complete attacker string remained data inside the escaped src value.
  • Benign regression fixture: all three patched surfaces loaded /assets/img/qrcode.png at 150x150, kept the correct link, and displayed the legacy stored filename audit&A.png as audit&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.
  • The same suite against the pinned upstream baseline: 0/10 pass.
  • node --check passes for both changed controller modules and the test file.
  • git diff --check 7b1c865..ca60455: passes.

The repository's existing npm run build does not complete in a clean checkout because its optimizer references the absent public/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.

@kian0417
kian0417 marked this pull request as draft October 8, 2026 01:34
@kian0417 kian0417 changed the title fix: escape attachment rendering (CVE-2024-7453) fix: prevent stored XSS in attachment tables (CVE-2024-7453) Oct 8, 2026
@kian0417
kian0417 marked this pull request as ready for review October 8, 2026 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant