[#36674] - [4.0]Inconsistency in MediaHelper::canUpload
- Closed
- 15 Jan 2022
- Medium
- Build: 4.0.6-dev
- # 36674
Steps to reproduce the issue
Code review of libraries/src/Helper/MediaHelper.php lines 243 - 252
// Media file names should never have executable extensions buried in them.
$executables = array_merge(self::EXECUTABLES, InputFilter::FORBIDDEN_FILE_EXTENSIONS);
// Remove allowed executables from array
if (count($allowedExecutables))
{
$executable = array_diff($executables, $allowedExecutables);
}
$check = array_intersect($filetypes, $executables);
Line 249 should be:
$executables = array_diff($executables, $allowedExecutables);
Additional comments
Compare the above to MediaHelper::checkFileExtension in lines 177 to 183:
// Remove allowed executables from array
if (count($allowedExecutables))
{
$executables = array_diff($executables, $allowedExecutables);
}
if (in_array($extension, $executables, true))
Votes
| Labels |
Removed:
?
|
||
joomla-cms-bot
- | Labels |
Added:
No Code Attached Yet
|
||
after going through the entire file I too think that line 249 should be changed
However, when I upload a restricted file type( E.g.- I tried uploading an SVG image, which is a restricted file type), considering this code snippet, I should not get an error, but I still get an error that I can't upload the file type
Can anyone explain to me why this happens?
@akshitrattan restricted file type and executable files are completely different things
@akshitrattan restricted file type and executable files are completely different things
oh, so as in a file like .exe??
joomla-cms/libraries/src/Helper/MediaHelper.php
Lines 28 to 40 in 65aaacc
joomla-cms/libraries/src/Helper/MediaHelper.php
Lines 28 to 40 in 65aaacc
thanks a lot, now I'm familiar with the exact meaning of the term
Who wants to make a pull request to fix that? I think the main challenge is to find good testing instructions for a real human test. As far as I can see there are no unit tests for the media helper yet, so I think it needs some kind of human test to make sure nothing is broken.
Who wants to make a pull request to fix that? I think the main challenge is to find good testing instructions for a real human test. As far as I can see there are no unit tests for the media helper yet, so I think it needs some kind of human test to make sure nothing is broken.
i'll make a PR
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-01-15 13:31:23 |
| Closed_By | ⇒ | richard67 |
From reading code I'd say you are right.