View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
12184 | Bug reports | Theme editor | public | 2017-03-08 15:41 | 2018-02-09 17:02 |
Reporter | Mazi | Assigned To | |||
Priority | none | Severity | minor | ||
Status | closed | Resolution | no change required | ||
Product Version | 2.64.x | ||||
Summary | 12184: Template editor fails to correctly detect file types | ||||
Description | When trying to upload a file called myjs.min.js this fails with the error message that the type is not allowed to be uploaded. But if I just upload a file called myjs.js this works fine. | ||||
Steps To Reproduce | Try uploading a file called xyz.min.js -> error. | ||||
Additional Information | This also applies to uploading: | ||||
Tags | No tags attached. | ||||
Bug heat | 6 | ||||
Complete LimeSurvey version number (& build) | 2.63.1+170305 | ||||
I will donate to the project if issue is resolved | No | ||||
Browser | Chrme | ||||
Database type & version | MySQL 5.5 | ||||
Server OS (if known) | Ubuntu 14 TLS | ||||
Webserver software & version (if known) | Apache 2.0 | ||||
PHP Version | 5.5.9 | ||||
This can be fixed by removing the period from the array of characters to be stripped on: https://github.com/LimeSurvey/LimeSurvey/blob/master/application/helpers/sanitize_helper.php#L114 Just a question of if there are any security implications of allowing multiple dots in a filename. Anyone have input on this? I noticed some other problems with this function: It does not handle multiple leading dots, only one. Need to change the regex from /^./ to /^.+/ on https://github.com/LimeSurvey/LimeSurvey/blob/master/application/helpers/sanitize_helper.php#L119 I have no idea what the if on https://github.com/LimeSurvey/LimeSurvey/blob/master/application/helpers/sanitize_helper.php#L121 is there for. It replaces the location of the last period with a period (which it already is.) However, if any of the statements between L115 and L121 change the length of the string, then this changes the wrong character to a period. @DenisChenu, you added this... do you remember what it's for? |
|
@mfavetti : too old here ..... But i think we must remove usage of this function, and use another for template. |
|
@mfavetti : do you work on it ? If not : i get it. |
|
It is not allowed because in the past some webserver software would have a securutiy vulnerability with double extensions. So you were able to run something like test.php.gif. |
|
Thanks forthe reminder : .min.js and .min.css seems great to be allowed (but can make without). |
|
Version 3.3.0 released |
|
Date Modified | Username | Field | Change |
---|---|---|---|
2017-03-08 15:41 | Mazi | New Issue | |
2017-03-08 22:03 | mfavetti | Note Added: 43237 | |
2017-03-09 08:22 | DenisChenu | Note Added: 43239 | |
2017-03-09 08:24 | DenisChenu | Note Added: 43240 | |
2017-03-09 11:43 | c_schmitz | Note Added: 43241 | |
2017-03-09 11:47 | c_schmitz | Note Edited: 43241 | |
2017-03-09 11:49 | DenisChenu | Note Added: 43242 | |
2018-02-09 12:13 |
|
Assigned To | => LouisGac |
2018-02-09 12:13 |
|
Status | new => resolved |
2018-02-09 12:13 |
|
Resolution | open => no change required |
2018-02-09 17:02 | c_schmitz | Note Added: 46356 | |
2018-02-09 17:02 | c_schmitz | Status | resolved => closed |