Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update $explodedName with reversed array #7814

Merged
merged 1 commit into from Sep 7, 2015

Conversation

@ryandemmer
Copy link
Contributor

commented Sep 4, 2015

With reference to issue #7813

array_reverse returns the reversed array, it does not change the original array.


Line 441 to 444 of libraries/joomla/filter/JInputFilter.php extracts possible extensions in the file name to check against a list of invalid extensions.

https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/filter/input.php

On line 442, array_reverse is used to re-arrange the array created so as to move the file name to the end, removing it with array_pop on line 443, but array_reverse returns the re-ordered array, it does not change the original array - http://php.net/manual/en/function.array-reverse.php

Therefore line 442 should be:

$explodedName = array_reverse($explodedName);

Update $explodedName with reversed array
With reference to issue #7813

array_reverse returns the reversed array, it does not change the original array.
@zero-24

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2015

Tested successfull with this script.

<?php
$intendedName = 'download.xml';
$explodedName = explode('.', $intendedName);
array_reverse($explodedName);
// $explodedName = array_reverse($explodedName);
array_pop($explodedName);
array_map('strtolower', $explodedName);

print_r($explodedName);

This return: Array ( [0] => download )

If we change the script to:

<?php
$intendedName = 'download.xml';
$explodedName = explode('.', $intendedName);
// array_reverse($explodedName);
$explodedName = array_reverse($explodedName);
array_pop($explodedName);
array_map('strtolower', $explodedName);

echo $explodedName;
print_r($explodedName);

It returns Array ( [0] => xml ).

Thanks.


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

@joomdonation

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2015

Test : Success.

The original code is clearly wrong and making file extension check doesn't work properly (instead of removing file name from array, it actually removes the file extension, so file extension is not being checked/validated with $options['forbidden_extensions']). This PR just correct it.

@zero-24

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2015

Setting RTC. Thanks


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

@joomla-cms-bot joomla-cms-bot added the RTC label Sep 7, 2015

@zero-24 zero-24 added this to the Joomla! 3.4.5 milestone Sep 7, 2015

@Kubik-Rubik Kubik-Rubik modified the milestones: Joomla! 3.4.4, Joomla! 3.4.5 Sep 7, 2015

@Kubik-Rubik

This comment has been minimized.

Copy link
Member

commented Sep 7, 2015

Thank you @ryandemmer! It's important enough to be merged in the RC version, merging by code review.

Kubik-Rubik added a commit that referenced this pull request Sep 7, 2015
Merge pull request #7814 from ryandemmer/patch-1
Update $explodedName with reversed array

@Kubik-Rubik Kubik-Rubik merged commit 805dbc3 into joomla:staging Sep 7, 2015

2 checks passed

JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joomla-cms-bot joomla-cms-bot removed the RTC label Sep 7, 2015

@810

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2015

We have now a regression on installing Kunena 3rt party templates:

Missing file to extract:

How to reproduce:

  • install kunena
  • download crypsisb3 www.kunena.org/download
  • go to backend - kunena - template manager - new template
  • select template and click on upload file and install

Extra info
our code: https://github.com/Kunena/Kunena-Forum/blob/develop/components/com_kunena/admin/controllers/templates.php#L98

@Bakual

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2015

@810 Reason is likely that you want to upload a file with a forbidden extension (eg ".php") then. That check was supposed to work since 3.4.0, but was broken.
If you use JFile::upload, you have to set the $allow_unsafe argument to true or explicitely pass $safeFileOptions with the correct options.

@joomdonation

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2015

Hmm

I have same issue with my extension. I think you will need to modify code in the line 103

file = $this->app->input->files->get('install_package');

To

file = $this->app->input->files->get('install_package', null, 'raw');

The reason is because this block of code https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/input/files.php#L81-L89

@Kubik-Rubik

This comment has been minimized.

Copy link
Member

commented Sep 9, 2015

As @Bakual said, this was a bug since 3.4.0 and was fixed finally in the version 3.4.4. Please use the raw parameter to suppress the safe file check.

@joomdonation

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2015

I just wanted to point out the needed changes. There are two changes needed:

  1. Use raw filter as I mentioned above.
  2. If we use JFile::upload, we will have to set the $allow_unsafe argument to true or explicitely pass $safeFileOptions with the correct options as @Bakual mentioned.
@Kubik-Rubik

This comment has been minimized.

Copy link
Member

commented Sep 9, 2015

@joomdonation Correct! Thank you for pointing it out.

@810

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2015

ok, thnx we had that before, only without the allow_unsafe. We changed that. Now its working again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.