Unit/System Tests PR-5.4-dev AI Generated Pending

User tests: Successful: Unsuccessful:

avatar XananasX7
XananasX7
22 May 2026

Summary

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.

Changes

  • Reject unsafe archive entry paths before extraction.
  • Reject absolute paths, stream wrappers, NUL bytes, Windows drive paths, and .. path segments.
  • Verify the final joined extraction path remains under the configured extraction root.
  • Reject symlink targets that resolve outside the extraction root.
  • Add unit coverage for traversal rejection, safe extraction, and unsafe symlink rejection.

Validation

  • php -l administrator/components/com_joomlaupdate/extract.php
  • php -l tests/Unit/Component/Joomlaupdate/Administrator/UpdateExtractionTest.php
  • git diff --check HEAD~1..HEAD
  • Standalone extractor harness confirmed:
    • traversal entry is rejected and does not write outside
    • regular archive entry still extracts
    • symlink target outside the root is rejected
avatar XananasX7 XananasX7 - open - 22 May 2026
avatar XananasX7 XananasX7 - change - 22 May 2026
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 May 2026
Category Administration com_joomlaupdate Unit Tests
avatar XananasX7 XananasX7 - change - 22 May 2026
Labels Added: Unit/System Tests PR-5.4-dev
avatar SniperSister
SniperSister - comment - 24 May 2026

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?

  1. the archive in question is provided by the Joomla project, a secure update payload is guranteed by the TUF signature.
  2. once the update has been applied, the code is executed - and that would include any malicious code that was part of the package. Considering that, why bothering with an out of root write request during updates if the write could be performed way easier after the update?
avatar XananasX7
XananasX7 - comment - 24 May 2026

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:

  1. close this as not worth the maintenance cost, or
  2. reduce it to a much smaller hardening patch that only rejects obvious unsafe archive names such as absolute paths, .. segments, stream wrappers, and unsafe symlink targets?

I’m happy to simplify the change significantly if a minimal extractor guard would still be useful.

avatar XananasX7
XananasX7 - comment - 24 May 2026

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.

avatar XananasX7
XananasX7 - comment - 31 May 2026

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.

avatar SniperSister
SniperSister - comment - 31 May 2026

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!

avatar XananasX7
XananasX7 - comment - 31 May 2026

Thanks @SniperSister — understood. Given the Super User trust boundary and the maintenance cost, I'll close this PR. Appreciate the thorough review.

avatar XananasX7 XananasX7 - change - 31 May 2026
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2026-05-31 06:44:20
Closed_By XananasX7
Labels Added: AI Generated
avatar XananasX7 XananasX7 - close - 31 May 2026

Add a Comment

Login with GitHub to post a comment