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

Cannot download msi file from download manager #4478

Closed
aducom opened this issue Apr 4, 2021 · 19 comments
Closed

Cannot download msi file from download manager #4478

aducom opened this issue Apr 4, 2021 · 19 comments
Labels
plugin: download status: awaiting feedback This issue may be fixed and is awaiting the original poster to confirm the fix. type: bug A problem that should not be happening
Milestone

Comments

@aducom
Copy link

aducom commented Apr 4, 2021

Bug Description

I have upgraded my website to the latest github. On my website users can download an msi file that is declared as an external file in the download manager. If you try to download then you get a 500 error message.

I thought it had to do with accepted filetypes and tried to assign it as a local file by trying to upload the msi. You can't. I couldn't find a place where I could define that except for uploads.

I can download pdf's so I think it has to do with the file type.

How to Reproduce

Steps to reproduce the behavior:
Just create a download of an msi and try to download

Expected Behavior

that i can download the file

Screenshots

just 500 (internal server error)

Server Information

e107
Version 2.3.1 (git)

Security level
[5] Balanced

Site Theme
hestia

Admin Theme
Bootstrap 3 v1.0 by e107 Inc (2013-12-25)

Install date
Wednesday 11 December 2019 - 08:32

Server
Apache/2
(host: www.phspeed.com)

PHP Version
7.4.14

MySQL
5.5.31
Database: aduc0madm_phspx
PDO: Enabled
Mode: NO_ENGINE_SUBSTITUTION

Charset
utf-8

Server Time
Sunday 04 April 2021 - 17:55


@aducom aducom added the type: bug A problem that should not be happening label Apr 4, 2021
@Moc
Copy link
Member

Moc commented Apr 4, 2021

Please check your Apache error log for more details (your hosting provider can help you with this). The error log provides more information.

I suspect it's a server configuration security measure.

@aducom
Copy link
Author

aducom commented Apr 4, 2021

I'll try to get it. Downloading worked fine in the original version so I'm curious ;-) I will install the backup on another location to verify as well.
But I doubt that this is a security issue, as I can download directly from the link, only not using the PHP download from E107.

@Jimmi08
Copy link
Contributor

Jimmi08 commented Apr 5, 2021

Hi,
there are 2 problems.

  1. manual uploading is not allowed although filetypes.xml is changed

  2. he uses external download so not uploading is not important now.

The problem is here:

Fatal error: Uncaught Error: Call to undefined function decorate_download_location() in C:\xampp8\htdocs\e107-gaia231\e107_plugins\download\request.php:224 Stack trace: #0 C:\xampp8\htdocs\e107-gaia231\e107_plugins\download\request.php(457): download_request::request() #1 C:\xampp8\htdocs\e107-gaia231\index.php(69): include_once('C:\\xampp8\\htdoc...') #2 {main} thrown in C:\xampp8\htdocs\e107-gaia231\e107_plugins\download\request.php on line 224

That file was changed from normal PHP to class and moved to plugin, but there is
e107::redirect(decorate_download_location($download_url));

but adding $this-> didn't help

The only fix was moving that function outside the download_request class.

@tgtje
Copy link
Contributor

tgtje commented Apr 5, 2021

This will be only a bug using php 8 (tested on local ) : (in addition to problem issue Jimmy08 mentioned, not at that point arrived)

Fatal error: Uncaught TypeError: count(): Argument #1 ($var) must be of type Countable|array, null given in R:\Xampp-803\htdocs\newest\e107_plugins\download\includes\admin.php:1821 Stack trace: #0 R:\Xampp-803\htdocs\newest\e107_plugins\download\includes\admin.php(418): download_main_admin_ui->submit_download() #1 R:\Xampp-803\htdocs\newest\e107_plugins\download\includes\admin.php(450): download_main_admin_ui->observe() #2 R:\Xampp-803\htdocs\newest\e107_handlers\admin_ui.php(1474): download_main_admin_ui->init() #3 R:\Xampp-803\htdocs\newest\e107_handlers\admin_ui.php(1447): e_admin_dispatcher->_initController() #4 R:\Xampp-803\htdocs\newest\e107_handlers\admin_ui.php(1333): e_admin_dispatcher->getController() #5 R:\Xampp-803\htdocs\newest\e107_handlers\admin_ui.php(1105): e_admin_dispatcher->runObservers() #6 R:\Xampp-803\htdocs\newest\e107_plugins\download\admin_download.php(244): e_admin_dispatcher->__construct() #7 {main} thrown in R:\Xampp-803\htdocs\newest\e107_plugins\download\includes\admin.php on line 1821

SInce MSI files contain some data code on front (which might be checked by/through server etc.. use FTP uload to import folder (filetypes adjusted and keep an eye out on 'naming' (space - and _ etc..).. This happens on a live php 7.4.10 site:
(upload through ftp (binary !!!), MM accept, as dl file. DL created.)

res1

edit : need to mention : not all hosters/servers accept the handling of executable files...(than store and provide in compressed form (zip/rar).

@aducom
Copy link
Author

aducom commented Apr 5, 2021

Actually I am uploading the MSI by using FileZilla and was using the external file. Jimako fixed the issue for me (I have bought the theme from him, great support), but I am running PHP 7.4 on the production server. But because of the issue I tried to upload the file using the regular way which failed. I did try to look into the htaccess, accepted filetypes, but I could not add the .msi to the accepted list of uploads, and I could not find where to do that. The MSI is also digitally signed, to avoid issues with browsers like Chrome and Edge and has been reported to several anti-virus manufacturers (fwiw). Browsers can behave strange sometimes ;-)
I don't know where you message is coming from, if you go to downloads and try to upload a file there, then you can only select files that are part of the supported list.
image

@Moc
Copy link
Member

Moc commented Apr 5, 2021

Alright, so it looks like it is a bug. Can you please check the Apache error log still? I am curious to see what it says. Or is that error that @Jimmi08 posted from the error log?

@aducom
Copy link
Author

aducom commented Apr 5, 2021

I suppose so, but I did ask the provider to give access to the logfiles. As it is Easter, I'm afraid that it might be problematic, because I cannot reproduce as Jimako fixed it. But we'll see.

@Jimmi08
Copy link
Contributor

Jimmi08 commented Apr 5, 2021

@Moc there are 2 issues... his original problem was due to that fatal error but you don't see it on the live server, so I tested it with his download table on localhost. That workaround helped.
But then he accidentally found the next problem - not able to upload msi file standard way.

@tgtje
Copy link
Contributor

tgtje commented Apr 6, 2021

Just adding imo : as i understand, the mimetype is in file class ; but is not in allowed types (eg for upload). Being an executable file it seems correct to me (sec. reason> local file upload vulnerability).

@aducom
Copy link
Author

aducom commented Apr 6, 2021

mmm, yes and no. I have a msi on my website for download to allow customers to download and install. The download is verified by Microsoft and Google, is digitally signed, and has all the marks for being a legit application.
Now download as an external file failed so I tried to do it the formal way and upload the file in a regular way. It failed.
Then there are some considerations. But first: my issue is solved in the old way so I don't have a problem currently.

  • If people install a CMS then that can be for multipurpose. If your site allows for uploads then it is up to the admin to decide what to do. I agree that the default setting should be as close as possible, but it should be possible to override.
  • There is always the way of uploading it externally, but it bypasses the standard E107 way. I can live with it, but it still is a work-around.

Point is, that I could not find a way of configuring the system so that it worked as expected. For advanced hackers, it is not that difficult to hide their stuff in 'legal' files. Perhaps off-topic, but still.

@tgtje
Copy link
Contributor

tgtje commented Apr 6, 2021

I understand, and partial agree; can (would like to ) say more, but it is posted as issue, adding more leads to discussion and distracts. ( i (pers.) leave it at that).

@CaMer0n CaMer0n added this to the e107 2.3.1 milestone Apr 6, 2021
CaMer0n added a commit that referenced this issue Apr 6, 2021
@CaMer0n
Copy link
Member

CaMer0n commented Apr 6, 2021

@aducom Normally this last commit should fix the download part of the issue. Uploading issue still requires a fix.

@CaMer0n
Copy link
Member

CaMer0n commented Apr 14, 2021

@aducom Would you mind retesting?

@aducom
Copy link
Author

aducom commented Apr 14, 2021 via email

@aducom
Copy link
Author

aducom commented Apr 26, 2021

@CaMer0n, which files do I need to replace? I don't want to do a full install due to some other mods. I have a bit of time now to look into it. I assume Issue #4478 - Fixes decorate_download_location() and check_download_l… Will start here.

@Moc
Copy link
Member

Moc commented Apr 26, 2021

@aducom
Copy link
Author

aducom commented Apr 26, 2021

I did. But actually I'm not sure what the modification did. The original issue was that I could not download an msi as an external file. That was fixed by Jane. The other issue is that I could not upload the msi as an accepted file and I could not add it to the list of accepted files. That is also in the download where you can upload an existing file from your local machine. Well, downloading the msi as an external file works fine with this file, so nothing changed there. But I still can't upload an msi.

@Moc
Copy link
Member

Moc commented Apr 26, 2021

Right, so you do actually need to update other files as well. You need these changes:

And then make sure filestypes.xml contains the msi extension (it is located in your e107_system(/hash) folder)

@Moc Moc added the status: awaiting feedback This issue may be fixed and is awaiting the original poster to confirm the fix. label May 3, 2021
@Moc
Copy link
Member

Moc commented Aug 30, 2021

Closing as presumed fixed. Please let us know if the issue still persists.

@Moc Moc closed this as completed Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: download status: awaiting feedback This issue may be fixed and is awaiting the original poster to confirm the fix. type: bug A problem that should not be happening
Projects
None yet
Development

No branches or pull requests

5 participants