[#29871] - [4.0] Add upload file name extension check to com_installer's Upload & Install function
- Closed
- 1 Jul 2020
- Medium
- Build: 4.0-dev
- Draft PR
- # 29871
- Diff
- richard67:4.0-dev-upload-file-name-check-installer
- Success Download Prebuilt packages are available for download. Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Success continuous-integration/drone/pr Build is passing Details
- Pending Hound Hound is busy sniffing around... Details
User tests: Successful: Unsuccessful:
Pull Request for #29763 (comment) (at least partly).
Summary of Changes
This Pull Request (PR) adds a check of the file name extension (or suffix) to the Upload & Install function of the Extensions Installer.
Currently only zip files work with Upload & Install, and so this PR will only allow files with extension (or suffix) ".zip" (case insensitive). If later more formats (".tar.gz", ".tar.bz2", ...) will be supported, the regular expression used for this new check can be easily extended by the new suffixes (has to be done at 2 places), or a server side check could check for the availability of the particular PHP modules and pass the regular expression to the js e.g. with a data- attribute or a hidden input field.
Important: This is NOT a security fix, it only shall prevent from accidently selecting the wrong file for upload and then having to wait until the upload completes just for getting an error message which is not really user friendly when unpacking the file fails. Further server-side checks, e.g. by mime type or checks of the content) have to be implemented separate from this PR with some future PR(s).
Testing Instructions
Will be added soon. Until this has been done I will leave this PR in draft status. As soon as draft status will be removed, the PR can be tested.
Actual result BEFORE applying this Pull Request
Will be added soon.
Expected result AFTER applying this Pull Request
Will be added soon.
Documentation Changes Required
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration Language & Strings JavaScript Repository NPM Change Front End Plugins |
Umm, last I remember, com_installer did NOT require only .zip files. So who changed this and why?
It can be that I got the code wrong and it supports other compression formats and extensions. @zero-24 can you maybe advise me?
The right fix for the original issue is server side content validation (i.e. MIME detection). Trying to do a “this file is safe to process” validation step based on uploaded file names is not reliable at all, file names and extensions are very easily faked whereas the file metadata is much more difficult to fake.
As stated in the description, this fix shall only prevent from accidently selecting the wrong file and not be a security fix.
Then use the native HTML feature which AFAIR everything from like IE10 (maybe 9) era support, which will let you tell the file browser what file types are allowed, and not allowed types will be shown in a disabled state and not selectable.
You’re doing file validation, no matter how you want to spin it. This JavaScript “helper” isn’t proper validation.
Then use the native HTML feature which AFAIR everything from like IE10 (maybe 9) era support, which will let you tell the file browser what file types are allowed, and not allowed types will be shown in a disabled state and not selectable.
Will check and do so. Please be patient because I currently have some private problems which require my time, too.
Thanks to @mbabker pointing me to the right way for the client-side pre-update check. I really appreciate this. It should be done with a new PR. Clsoing this one since it does it in a wrong silly way. Additional server-side validations I'll do with future PRs if nobody else is faster.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-07-01 12:14:14 |
| Closed_By | ⇒ | richard67 | |
| Labels |
Added:
?
NPM Resource Changed
?
|
||
Umm, last I remember, com_installer did NOT require only .zip files. So who changed this and why?
The right fix for the original issue is server side content validation (i.e. MIME detection). Trying to do a “this file is safe to process” validation step based on uploaded file names is not reliable at all, file names and extensions are very easily faked whereas the file metadata is much more difficult to fake.