thorsten/phpmyfaq: SVG upload: SvgSanitizer::sanitize() reassembles <script> and reports success, so a FAQ_EDIT user can store executable SVG

Summary

SvgSanitizer::removeDangerousContent() strips dangerous elements and patterns with one preg_replace pass each and never re-scans the result. preg_replace does not look at text it has just produced, so a payload that splits a dangerous tag around a token which is itself removed is reassembled by the removal:

uploaded: <scr<?php?>ipt>alert(document.domain)</scr<?php?>ipt>
stored:   <script>alert(document.domain)</script>

The processing-instruction pattern sits at index 21 of DANGEROUS_PATTERNS; the three <script> patterns sit at indices 0 to 2 and have already run.

sanitize() writes that output back and returns true. ImageController::upload() calls it precisely when isSafe() has already judged the file dangerous — it even logs "Potentially malicious SVG upload detected" — and keeps the file because cleaning "succeeded".

Affected versions

Confirmed on commit 9847140, the head of main when I tested.

The most recent change to this file, c886cc9 (2026-09-10, "reject SVG DTD custom entities in SvgSanitizer to prevent XSS"), is included in what I tested. The bypass survives it, including the variant that uses an entity declaration as the splitting token.

The 4.1 line is out of scope: phpmyfaq/src/phpMyFAQ/Helper/SvgSanitizer.php does not exist on the 4.1 branch, so neither the sanitizer nor the upload branch that calls it is there. I have not tested any released ta rball of the 4.2 line, only the commit above.

Preconditions

An application is affected only if all of the following are met:

• SVG upload is reachable. svg => image/svg+xml is in ImageController::ALLOWED_MIME_TYPES as shipped, so this IS the default.
• The attacker holds an account with PermissionType::FAQ_EDIT. I tested with a user granted only edit_faq and view_faqs; adduser, editconfig and addnews all return false for it.
• The attacker can read a pmf-csrf-token, which the FAQ overview page renders for that account.
• content/user/images/ is served by the web server. Your Apache, nginx and Caddy configs all block content/core/, content/user/attachments/ and content/*.zip, but none of them block this directory, and none set a Content-Security-Policy. The production Caddyfile sets X-Content-Type-Options: nosniff, which does not stop a file already declared image/svg+xml from running script.

I am listing the FAQ_EDIT requirement as a limitation of this report rather than leaving you to find it: this is privilege escalation from content editor to script execution in the site origin, not an unauthenticated issue. Reading the stored file needs no account at all.

Reproduction

Against a running instance at $BASE as a user holding only edit_faq (setup in Test environment). Every step is a real request.

1. Write the payload to lexa.svg:

<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><scr<?php?>ipt>alert(document.domain)</scr<?php?>ipt></svg>

1. Log in:

curl -s -c jar -b jar -o /dev/null -w '%{http_code}\n' -X POST "$BASE/admin/authenticate" \
  --data-urlencode 'faqusername=editor' --data-urlencode 'faqpassword=<password>'

Expected: 302.

1. Read the CSRF token. The FAQ overview is reachable with edit_faq; /admin/faq/add requires add_faq and answers 403.

TOKEN=$(curl -s -c jar -b jar "$BASE/admin/faqs" | sed -n 's/.*name="pmf-csrf-token" value="\([0-9a-f]*\)".*/\1/p' | head -1)

1. Upload:

curl -s -c jar -b jar -X POST "$BASE/admin/api/content/images?csrf=$TOKEN" -F 'files[]=@lexa.svg;type=image/svg+xml'

Expected: {"success":true, … "files":["…/content/user/images/<timestamp>_lexa.svg"] … "code":220}, and in the log, at the same moment, Potentially malicious SVG upload detected: <file> by user 2. The file was recognised as dangerous and kept anyway.

1. Fetch it back with no cookies:

curl -s -i "$BASE/content/user/images/<timestamp>_lexa.svg"

Expected: 200, Content-Type: image/svg+xml, and a body identical to the payload except that <scr<?php?>ipt> has become <script>.

1. Open that URL in a browser. The script runs: an alert shows the site host, and the page origin is the application's own origin. A screenshot is attached.

<img width="1100" height="700" alt="20260913-2339-phpmyfaq-svg-alert" src="https://github.com/user-attachments/assets/5966a55f-9a32-4b0c-a8d3-6796db718c45" />

Impact

Measured end to end on a running instance: a user whose only rights are edit_faq and view_faqs uploaded the file above, the API answered success: true, and the file became readable with no session. Fetched anonymously it returns image/svg+xml with an unmodified <script>alert(document.domain)</script>. Opened in a browser it executed, in the application's origin. phpMyFAQ logged the upload as potentially malicious and stored it regardless.

Inferred, not measured: script in that origin reaches whatever the viewing session reaches, including an administrator's if an administrator opens the link. I did not attempt session theft or any administrative action. Embedding the image in FAQ content does not fire on its own — the editor inserts it with <img>, which does not run scripts — so the victim has to open the file URL.

Severity

Entered in the Severity field as CVSS:3.1/AV:N/AC:L/PR:L/UI:R/S:C/C:L/I:L/A:N (Base 5.4).

PR:L reflects the FAQ_EDIT requirement, UI:R that the victim must open the stored file rather than merely view a FAQ page, and S:C the usual treatment of stored XSS. If you weigh the administrator-session case more heavily than I have, C:H is a defensible reading. Final scoring is of course yours.

Root cause

phpmyfaq/src/phpMyFAQ/Helper/SvgSanitizer.php:315 — removeDangerousContent() applies each removal exactly once and does not iterate to a fixed point, so text produced by one removal is only ever examined by patterns later in the same sequence. CWE-182 (Collapse of Data into Unsafe Value).

animate and set compound this: they are in DANGEROUS_ELEMENTS but in none of DANGEROUS_PATTERNS, so once reassembled at stage two, no later stage looks at them again.

Suggested fix

Any one of these closes the reported inputs; they differ in how much they change.

The smallest is at the call site. ImageController.php:147 already knows the file is dangerous — it logs exactly that — and then trusts sanitize() to repair it. Deleting the upload instead would not depend on the sanitizer being complete, and shouldReject() already exists.

Alternatively, iterate removeDangerousContent() until the string stops changing, bounded by an iteration cap; decodeAllEntities() in the same class already uses that shape. Or re-run the isSafe() checks on the sanitized output and return false when anything dangerous remains.

Adding patterns does not close the class: removal itself can construct a new tag, so any list can be split around its own entries. I do not know which of the three fits your compatibility constraints best.

Not verified

• 4.1.x and released tarballs: not tested, branches not read.
• Apache: I ran nginx, your default preset. On the Apache profile the shipped .htaccess sent /admin/ into a rewrite loop in my environment before I reached the upload. That its rules leave content/user/images/ served is a code reading, not a test.
• I did not attempt session theft, administrative actions, or any escalation beyond observing the alert.
• I did not audit other callers of SvgSanitizer; there may be more entry points than the one I used.
• I did not enumerate the splitting tokens. A processing instruction and an entity declaration both work, and animate survives without one because no pattern re-checks it.
• One browser only (Chromium 153).

Test environment

phpMyFAQ at commit 9847140, installed headlessly; your Docker stack (nginx 1.31, php-fpm on PHP 8.5, MariaDB 11); Chromium 153 on Debian 13.

To reproduce from scratch:

git clone https://github.com/thorsten/phpMyFAQ.git && cd phpMyFAQ
git checkout 9847140 && composer install
bin/dev up nginx mariadb
php bin/console phpmyfaq:install --db-type=mysqli --db-server=db \
  --db-user=phpmyfaq --db-password=phpmyfaq --db-name=phpmyfaq \
  --admin-user=admin --admin-password=<pw> --base-url=$BASE --force

Then, as the administrator, create a user editor and grant it edit_faq and view_faqs only.

The development stack is what I had to hand, but nothing here depends on it: the code path has no debug branch, and the production Apache and Caddy configs block the same three paths as the development ones while leaving content/user/images/ served and setting no CSP. If the content directory is not writable by the web server user, the install and the upload both fail with a permission error rather than the behaviour above.

Scope

Your policy excludes findings from automated scanners without demonstrated exploitable impact. The function was surfaced by a static-analysis tool I am working on, but everything above is a recorded request or response against a running instance, ending in an observed script execution.

Your policy also excludes issues needing non-default configuration. None is involved: SVG upload is enabled as shipped, and the account holds no rights beyond editing FAQs.

I checked for duplicates first. No issue or pull request in this repository mentions it, and no published advisory covers SVG upload; the closest, GHSA-x6qj-5jhf-xgpm, is stored XSS via html_entity_decode() in FAQ rendering, a different component and mechanism.

I have not used this against any system I do not own, and I will not publish anything without agreeing a date with you first. Your 90-day default suits me; tell me if you need longer.