Skip to content

[5.4] Fix path traversal in com_templates file operations#47851

Open
ManhThuan wants to merge 1 commit into
joomla:5.4-devfrom
ManhThuan:security/path-traversal-com-templates
Open

[5.4] Fix path traversal in com_templates file operations#47851
ManhThuan wants to merge 1 commit into
joomla:5.4-devfrom
ManhThuan:security/path-traversal-com-templates

Conversation

@ManhThuan
Copy link
Copy Markdown

Add Path::check() validation to file operation methods in TemplateModel
that were missing it, consistent with getSource() which already uses it.

Affected methods: save(), deleteFile(), renameFile(), cropImage(), resizeImage()
all accepted a base64-encoded file parameter without validating the decoded
path stays within the template directory, allowing directory traversal via
sequences such as /../../../ to read, write, rename or delete files anywhere
under JPATH_ROOT.

  • I read the Generative AI policy
    and my contribution is either not created with the help of AI or is
    compatible with the policy and GNU/GPL 2 or later.

Summary of Changes

Path::check() validation has been added to five file operation methods in
TemplateModel that were missing it. Path::clean() alone only normalises
path separators; it does not prevent ../ directory traversal. Path::check()
resolves the path and verifies it stays within JPATH_ROOT, throwing an
exception on traversal attempts.

Methods patched: save(), deleteFile(), renameFile(), cropImage(),
resizeImage(). The fix follows the pattern already used by getSource() in
the same class (line 945).

Testing Instructions

  1. Log in as an Administrator with Template Manager access.
  2. Open the template editor for any installed template
    (e.g. Cassiopeia): System → Site Templates → Cassiopeia → Edit.
  3. In the browser address bar, find the file parameter (base64-encoded).
  4. Replace it with base64_encode("/../../../configuration.php").
  5. Submit a save or delete action.

Before patch: the operation proceeds on configuration.php outside the
template directory.
After patch: Joomla displays an error message and refuses the operation.

Actual result BEFORE applying this Pull Request

File operations (save, deleteFile, renameFile, cropImage, resizeImage)
in the template editor accept a base64-encoded path parameter that is decoded
and used without traversal validation. A /../ sequence in the decoded value
allows the operation to target any file under JPATH_ROOT, including
configuration.php and files in libraries/, plugins/, etc.

Expected result AFTER applying this Pull Request

All five file operation methods reject paths containing ../ sequences with an
error message, confining template file operations to the template directory.
Behaviour is now consistent with getSource(), which already applied
Path::check().

Link to documentations

  • No documentation changes for guide.joomla.org needed
  • No documentation changes for manual.joomla.org needed

Add Path::check() validation to file operation methods in TemplateModel
that were missing it, consistent with getSource() which already uses it.

Affected methods: save(), deleteFile(), renameFile(), cropImage(), resizeImage()
all accepted a base64-encoded file parameter without validating the decoded
path stays within the template directory, allowing directory traversal via
sequences such as /../../../ to read, write, rename or delete files anywhere
under JPATH_ROOT.
@brianteeman
Copy link
Copy Markdown
Contributor

If you have access to the template manager (restricted to super users) then you really can do anything you want

@bembelimen
Copy link
Copy Markdown
Contributor

Thank you for your contribution, but please stop submitting AI PR proposals. See the AI policy you checked and hopefully read before. We're looking for thoughtful contributions. See this as a first warning.

@laoneo
Copy link
Copy Markdown
Member

laoneo commented May 29, 2026

In general it is not a bad idea to restrict the template manager to edit files only in its own directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants