View Issue Details

Jump to NotesJump to History
This bug affects 1 person(s).
 6
IDProjectCategoryView StatusDate SubmittedLast Update
12184Bug reportsTheme editorpublic2017-03-08 15:412018-02-09 17:02
ReporterMazi Assigned ToLouisGac 
PrioritynoneSeverityminor 
Status closedResolutionno change required 
Product Version2.64.x 
Summary12184: 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.
I assume the duplicate "." within the file name causes the problems so the ending isn't detected correctly.

Steps To Reproduce

Try uploading a file called xyz.min.js -> error.
Rename the file to xyz.js -> That should work fine.

Additional Information

This also applies to uploading:
style.min.css -> fail
style.css -> works fine

TagsNo tags attached.
Bug heat6
Complete LimeSurvey version number (& build)2.63.1+170305
I will donate to the project if issue is resolvedNo
BrowserChrme
Database type & versionMySQL 5.5
Server OS (if known)Ubuntu 14 TLS
Webserver software & version (if known)Apache 2.0
PHP Version5.5.9

Users monitoring this issue

There are no users monitoring this issue.

Activities

mfavetti

mfavetti

2017-03-08 22:03

developer   ~43237

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?
DenisChenu

DenisChenu

2017-03-09 08:22

developer   ~43239

@mfavetti : too old here ..... But i think we must remove usage of this function, and use another for template.
DenisChenu

DenisChenu

2017-03-09 08:24

developer   ~43240

@mfavetti : do you work on it ? If not : i get it.
c_schmitz

c_schmitz

2017-03-09 11:43

administrator   ~43241

Last edited: 2017-03-09 11:47

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.
I think you could mitigate this if you add 'min' to the allowed extensions and on upload you take teh file apart and check each extension by itself.
DenisChenu

DenisChenu

2017-03-09 11:49

developer   ~43242

Thanks forthe reminder : .min.js and .min.css seems great to be allowed (but can make without).
c_schmitz

c_schmitz

2018-02-09 17:02

administrator   ~46356

Version 3.3.0 released

Issue History

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 LouisGac Assigned To => LouisGac
2018-02-09 12:13 LouisGac Status new => resolved
2018-02-09 12:13 LouisGac 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