Skip to content

Widen the upload blocklist to the executable spellings it forgot - #48

Merged
AllTerrainDeveloper merged 1 commit into
mainfrom
harden/upload-blocklist-forminator-lesson
Aug 29, 2026
Merged

AllTerrainDeveloper merged 1 commit into
mainfrom
harden/upload-blocklist-forminator-lesson

Conversation

@AllTerrainDeveloper

Copy link
Copy Markdown
Owner

Why

Prompted by the Forminator arbitrary file upload write-up (CVE-2026-15748, CVSS 9.8, ~600k sites). I audited our upload path against its two root causes.

We share neither root cause — this PR is defense-in-depth, not a fix for an active hole.

Forminator root cause Us
Public handler trusted upload-field config from the request (forged via a Select field value) alltfo_handle_uploads() reads filetypes/maxsize/maxfiles only from alltfo_get_form_schema($form_id) — the stored form. The request supplies bytes, never configuration.
Dangerous-extension blocklist did exact-key matching against pipe-joined MIME keys (php|php3|…) Our check matches one real pathinfo extension against a flat set — no alternation to hide inside — then wp_check_filetype_and_ext() sniffs bytes, wp_handle_upload() runs with a mimes map scoped to the field, files land under a hashed name in a Require all denied directory as private attachments.

Also confirmed: no standalone/nopriv upload endpoint (Forminator's handle_file_upload was directly reachable); ours only runs inside the authenticated-or-not submission pipeline, after nonce/spam gating.

What changed

The blocklist claimed to cover "anything the web server might execute" but omitted several real spellings. Added: pht, phtm, php1, php2, php6, phpt, hphp, and the SSI trio shtml/shtm/stm. These were already caught by the byte check + deny-all directory in practice, but the unconditional first gate shouldn't rely on the layers behind it to cover a missing spelling.

Tests

New tests/phpunit/tests/uploads.php (4 tests, 37 assertions):

  • every executable extension refused even when the field's own allow-list names it (blocklist overrides allow-list, and isn't filterable)
  • double extension evil.php.jpg caught by the byte check
  • the Forminator lesson in one assertion: accepted types come from the schema; a forged filetypes in the request changes nothing
  • a legitimate .txt is accepted and stored private

744 PHPUnit ✓ · PHPCS 0 errors

🤖 Generated with Claude Code

https://claude.ai/code/session_01DVAaHFURDGeBzeuWHZ2dcP

The Forminator arbitrary-upload bug (CVE-2026-15748) turned on two things:
a handler that trusted upload-field configuration from the request, and a
dangerous-extension check that matched pipe-joined MIME keys with exact
comparison. We share neither — the accepted types come from the stored
schema, and the check matches one real extension against a flat set — but
that flat set was not as complete as it claimed.

Add the executable spellings a bypass reaches for precisely because a
shorter list forgets them: pht, phtm, php1/php2/php6, phpt, hphp, and the
SSI trio shtml/shtm/stm. Even without them a crafted name was caught by the
byte check and the deny-all directory; a blocklist that is the unconditional
first gate should not lean on the layers behind it to cover a spelling.

With a dedicated upload-security suite pinning both defenses: every
executable extension refused even when the field's own allow-list names it,
a double extension caught by the byte check, and — the Forminator lesson in
one assertion — accepted types read from the schema while a forged config in
the request changes nothing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVAaHFURDGeBzeuWHZ2dcP
@AllTerrainDeveloper
AllTerrainDeveloper merged commit aa12e4a into main Aug 29, 2026
3 checks passed
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