PR-staging RTC

Pending

User tests: Successful: Unsuccessful:

avatar nonickch
nonickch
16 May 2019

Pull Request for Issue ##24539 and #24723 .

Summary of Changes

Fix directory and file traversal/creation/deletion in com_media for a symlinked "images" folder.
See original bug submission at #24539, PR #24723 by @smehrbrodt and extra analysis by @HLeithner .
Please note that the #24723 changes are included in this PR although not via a merge that shows history (simply copy/paste of the diffed line).

Testing Instructions

Have the /images directory as a symlink that points outside the J installation folder. In my test case it points to ../images
1.Through the back-end mediamanager (/administrator/index.php?option=com_media&folder=), try to navigate to a subdirectory by clicking on the subdirectory icon.
2. Through the back-end mediamanager (/administrator/index.php?option=com_media&folder=), try to upload an image by clicking the "+Upload" button in the toolbar.
3. Through the back-end mediamanager (/administrator/index.php?option=com_media&folder=), try to delete an image by clicking the "X" icon at the top-left of a listed image.
4. Through the back-end mediamanager (/administrator/index.php?option=com_media&folder=), try to create a new directory by clicking the "Create New Folder" button in the toolbar.
5. Through the back-end mediamanager (/administrator/index.php?option=com_media&folder=), try to delete a directory by clicking the "X" icon at the top-right of the folder icon.
6. While in the back-end editing an article with TinyMCE (/administrator/index.php?option=com_content&view=article&layout=edit), drag&drop an image from your OS file manager in the content editing area

Expected result

  1. UI directory traversal should display the contents of the selected directory.
  2. Image uploads take place
  3. Image deletions take place
  4. Directory creations take place
  5. Directory deletions take place
  6. Image upload via the drag&drop takes place and the image is inserted in the content area

Actual result

  1. The pageload proceeds without any apparent errors yet it still shows the contents of the root /images directory.
  2. The file upload fails. No image upload takes place and a red Error box with the message "Invalid folder provided." appears
  3. The file deletion fails. No image deletion takes place and a red Error box with the message "Unable to delete: [uploaded-filename]. File name must only have alphanumeric characters and no spaces." appears
  4. The directory creation fails. No directory gets created and a green Info box appears with the message "Unable to create folder. Folder name must only have alphanumeric characters and no spaces.".
  5. The directory deletion fails. No directory deletion takes place and a red Error box with the message "Invalid folder provided." appears
  6. An upload begins and fails with the error message: "Unable to upload file.: [realpathed-base-media-dir]undefined". No file gets uploaded.

Documentation Changes Required

None

avatar nonickch nonickch - open - 16 May 2019
avatar nonickch nonickch - change - 16 May 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 May 2019
Category Administration com_media
avatar wnkkm
wnkkm - comment - 18 May 2019

Can the fix also work for a symlinked sub-folder in Joomla installation's 'images' folder?
( J-installation/images/media --symlink--> /media )
This case is a bit different from the test scenario.

Thanks.

avatar nonickch nonickch - change - 18 May 2019
Labels Added: PR-staging
avatar nonickch
nonickch - comment - 18 May 2019

No, unfortunately this is only going to work for the upper images (e.g COM_MEDIA base) dir.
As discussed in #24539 that'd need something different than realpath() to perform the path checks, and the proposed alternative/code was met with some resistance over "over-engineering".

I'd be happy to go down that path, but I fear that PR would be in danger getting rejected.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 May 2019

I've tested this and I'm fine with it as it solves my immediate problem. I understand that there remain edge cases which are not solved but, in my opinion, those can be solved by a later patch (if at all).

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 21 May 2019

@okonomiyaki3000 can you please mark your test as successfully at Issue Tracker?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 May 2019

@franz-wohlkoenig is there any link to issue tracker around here?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 May 2019

👍


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24924.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 May 2019

I have tested this item successfully on 5ae54c7

👍


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24924.

avatar okonomiyaki3000 okonomiyaki3000 - test_item - 21 May 2019 - Tested successfully
avatar SharkyKZ SharkyKZ - test_item - 23 May 2019 - Tested successfully
avatar SharkyKZ
SharkyKZ - comment - 23 May 2019

I have tested this item successfully on 5ae54c7


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24924.

avatar SharkyKZ SharkyKZ - test_item - 23 May 2019 - Tested successfully
avatar franz-wohlkoenig franz-wohlkoenig - change - 23 May 2019
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 23 May 2019

Status "Ready To Commit".

avatar nonickch nonickch - change - 23 May 2019
Labels Added: RTC
avatar HLeithner HLeithner - close - 5 Jun 2019
avatar HLeithner HLeithner - merge - 5 Jun 2019
avatar HLeithner
HLeithner - comment - 5 Jun 2019

thx

avatar HLeithner HLeithner - change - 5 Jun 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-06-05 15:18:57
Closed_By HLeithner

Add a Comment

Login with GitHub to post a comment