Skip to content

Folder or file could start with dot#33151

Closed
n3t wants to merge 1 commit intojoomla:stagingfrom
n3t:Path-Input-Filter
Closed

Folder or file could start with dot#33151
n3t wants to merge 1 commit intojoomla:stagingfrom
n3t:Path-Input-Filter

Conversation

@n3t
Copy link
Contributor

@n3t n3t commented Apr 15, 2021

Files and folders starting with dot didn't pass the regexpr test. So for example \var\www.tmp path didn't pass the test, which causes error during Joomla update.

Summary of Changes

Modified Reg Expressions to include files and / or folders starting with dot. The check was brought by this PR #32076 which adds InputFilter to temp path settings, however ignoring folders with dot at the beginning.

Testing Instructions

Go to Joomla settings and change your Temp path to some folder with dot at the beginning, for example \path_to_joomla.tmp (and create that folder of course). Go to Joomla update, and try to install the update.

Actual result BEFORE applying this Pull Request

Error is displayed, update is not installed.

Expected result AFTER applying this Pull Request

No errror, update is installed.

Documentation Changes Required

None

Files and folders starting with dot didn't pass the regexpr test. So for example \var\wwww\.tmp path didn't pass the test, which causes error during Joomla update.
@brianteeman
Copy link
Contributor

Files and folders starting with dot didn't pass the regexpr test.

thats good as they are not supposed to

@PhilETaylor

This comment was marked as abuse.

@n3t
Copy link
Contributor Author

n3t commented Apr 15, 2021

@brianteeman There is no reason that those files and folders should be ommited. Especially folders. For example ".tmp" makes that folder hidden on Linux, not visible for example through FTP access.

@PhilETaylor

This comment was marked as abuse.

@n3t
Copy link
Contributor Author

n3t commented Apr 15, 2021

@PhilETaylor I know you can switch on to show it, but if you just want to keep your view clean, you can easily hide it with dot.

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@n3t
Copy link
Contributor Author

n3t commented Apr 15, 2021

libraries/src/Filter/InputFilter.php only calll parent::clean function. To modify it would mean to copy all the code from libraries/vendor/joomla/filter/src/InputFilter.php...

How to get comment from @joomla/security team? Here, or need to be contacted other way?

@PhilETaylor

This comment was marked as abuse.

@n3t
Copy link
Contributor Author

n3t commented Apr 15, 2021

Ok, thanks. The PR should be propably made to https://github.com/joomla-framework/filter package, or as you proposed, to 'libraries/src/Filter/InputFilter.php', or maybe directly to Model of Joomla Update component. I contacted security team to review this and give advice.

Generally this is related to issue you mentioned here #32567 but on other place of Joomla.

@richard67
Copy link
Member

richard67 commented Apr 16, 2021

@n3t Your pull request is wrong for following reasons:

  1. Your regular expressions are wrong. They will, in opposite to those in current Joomla 3 code, allow paths like e.g. /some/folder/../../../evil.php on Linux, which means breaking out of the Joomla root. This open a security hole and so cannot be accepted. You can easily very that with one of the many available sites for online execution of preg_match. Just Google for it.
  2. Any change has to be made in the https://github.com/joomla-framework/filter repository .

I suggest you open an issue with a feature request in the https://github.com/joomla-framework/filter repository. The feature request should clearly describe your requirement (to be able to use folder and file names starting with a dot).

And please don't contact anymore the SST for discussing this PR via their email for reporting security issues. The form which you have used is ONLY for reporting security issues. @PhilETaylor had already notified the right person in his comment here #33151 (comment) .

@richard67 richard67 closed this Apr 16, 2021
@n3t
Copy link
Contributor Author

n3t commented Apr 16, 2021

@richard67 Thanks for your review, sorry for using contact form, I did it just because asked to. I will raise an issue on framework. Anyway, checking double dots in tmp folder set in configuration.php doesn't bring any real security in Joomla update process, as I can easily go to configuration, and set directly /other/folder/evil.php, which will pass the test as correct path...

@richard67
Copy link
Member

@n3t We don't protect a super user from himself (or herself). If you enter such an absolute path in backend you should know what you are doing. There are other ways to manipulate paths than changing in backend.

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