User tests: Successful: Unsuccessful:
Tighten the standalone Joomla Update ZIP extractor so archive entries cannot write outside the configured extraction directory.
The extractor prepends the Joomla root to local ZIP entry names, but it did not validate archive member paths before creating directories or opening files. A crafted ZIP entry such as ../outside.txt could
be extracted outside the intended destination.
.. path segments.php -l administrator/components/com_joomlaupdate/extract.phpphp -l tests/Unit/Component/Joomlaupdate/Administrator/UpdateExtractionTest.phpgit diff --check HEAD~1..HEAD| Status | New | ⇒ | Pending |
| Category | ⇒ | Administration com_joomlaupdate Unit Tests |
| Labels |
Added:
Unit/System Tests
PR-5.4-dev
|
||
Thank you for the review.
That threat model makes sense if this extractor only processes Joomla update packages after successful TUF verification. My goal here was defense-in-depth for the archive extraction boundary, not to claim
this bypasses TUF or creates an independent remote exploit against normal update flow.
I agree the current patch may be too large if TUF is considered the security boundary for this code path. Would you prefer that I:
.. segments, stream wrappers, and unsafe symlink targets?I’m happy to simplify the change significantly if a minimal extractor guard would still be useful.
Thanks, that makes sense. I rechecked the current flow.
The only non-TUF attacker-controlled path I can identify is the manual Upload & Install update flow, but that requires a Super User and captive re-authentication. For the normal update flow, the package is
downloaded from TUF metadata and the local archive hash is checked before extraction.
Given that, I agree this is better framed as defense-in-depth hardening rather than an independent vulnerability. If the project does not want the added complexity, I’m fine closing this PR or reducing it to
a smaller path-normalization guard.
Thanks again for the review @SniperSister.
To clarify the concrete attack surface after re-reading the code more carefully:
The upload() path in UpdateModel.php (lines 1202–1267) accepts an arbitrary ZIP uploaded by a Super User and stores it in tmp. That file then feeds into extract.php/ZIPExtraction via the subsequent AJAX extraction call — without TUF verification (TUF only covers the auto-download path from the update server). The attacker in this scenario is the Super User themselves.
You are absolutely right that a Super User already has the keys to the kingdom — they can upload PHP files via com_media or the extension installer anyway. So the severity of the upload-path traversal is low (requires Super User + deliberate malice).
However, the fix is still valuable as defense-in-depth at the extraction boundary, following the principle of least privilege: even if a Super User uploads a crafted ZIP, the extractor should not write files outside $addPath. It prevents accidental or edge-case traversal by a confused/compromised Super User or a poorly constructed archive.
If the project's policy is that defense-in-depth hardening in the extractor is not worth the maintenance cost given the Super User trust level, I completely understand and will close this PR. Just wanted to make sure the concrete code path was clear.
If the project's policy is that defense-in-depth hardening in the extractor is not worth the maintenance cost given the Super User trust level, I completely understand and will close this PR. Just wanted to make sure the concrete code path was clear.
I'm not a maintainer of the CMS and therefore can't make the final call, I can only give you my personal opinion: it's not worth it.
Nevertheless: thank you for the contribution and the effort, it's very much appreciated!
Thanks @SniperSister — understood. Given the Super User trust boundary and the maintenance cost, I'll close this PR. Appreciate the thorough review.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2026-05-31 06:44:20 |
| Closed_By | ⇒ | XananasX7 | |
| Labels |
Added:
AI Generated
|
||
Thank you for your contribution!
I see what you are trying to achieve, however I’m questioning if the extra code, complexity and maintenance impact is worth the benefit.
Why is that?