View Issue Details

This bug affects 1 person(s).
 16
IDProjectCategoryView StatusLast Update
08818Bug reportsOtherpublic2014-03-21 09:47
Reporterhermann Assigned ToDenisChenu  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version2.05+ 
Fixed in Version2.05+ 
Summary08818: Comparison operators (< and >) get converted to HTML-entities in expressions in questions
Description

If an expression using a comparison operator is entered into the text of a question ">" and "<" get converted into ">" and "<" breaking the expression.

This can only be avoided by becoming an admin-user or by disabling xssfiltering.

Steps To Reproduce

create survey
create question-group
create question
insert e.g.

{if(1 < 2,"less","more")}

into question.
save
check survey logic for current question

TagsNo tags attached.
Bug heat16
Complete LimeSurvey version number (& build)140302
I will donate to the project if issue is resolvedNo
Browser
Database type & versionMySQL Ver 14.14 Distrib 5.1.61
Server OS (if known)CentOS release 6.2 (Final)
Webserver software & version (if known)Apache 2.2.15
PHP Version5.3.3

Relationships

has duplicate 08880 closedDenisChenu Feature requests EM validation shows error using > < operators 
related to 08887 closedDenisChenu Bug reports Question text / admin : unable to use & gt ; (and a a lt ;) 

Users monitoring this issue

There are no users monitoring this issue.

Activities

hermann

hermann

2014-03-06 15:26

reporter   ~29119

Is this a regression of http://bugs.limesurvey.org/view.php?id=6536 ?

DenisChenu

DenisChenu

2014-03-06 15:52

developer   ~29120

Last edited: 2014-03-06 15:53

http://manual.limesurvey.org/Expression_Manager#Security_issue
{if(1 lt 2,"less","more")}

ELse , need to find a way :)

Mazi

Mazi

2014-03-06 15:53

updater   ~29121

Disabling the HTML/CSS filter at global settings -> security should solve this issue though I agree that this is no real fix but should be fixed at the Limesurvey code if possible.

DenisChenu

DenisChenu

2014-03-06 16:04

developer   ~29122

Better is to use lt,le,gt and ge (no need to disable xss) . But yes, try to find a fix.

hermann

hermann

2014-03-06 16:09

reporter   ~29123

Just my two cents:
As far as I can tell the code checks questions with an HTMLPurifier before doing anything else. How about first validating it (or at least containing expressions) with the "expression-manager" (if such a thing exists in the code). If an expression fails that validation it can/should be purified...

DenisChenu

DenisChenu

2014-03-06 16:11

developer   ~29124

No,

Because we need to purify
<script>alert('XSS')</script>{SOMETHING}

Or
{if(1 lt 2,"<span onmouseover='alert(XSS)'>mouse hover</span>","more")}

hermann

hermann

2014-03-06 16:36

reporter   ~29126

Last edited: 2014-03-06 16:37

Hi DenisChenu,

I know why this purification is necessary. But
{if(1 lt 2,"<span onmouseover='alert(XSS)'>mouse hover</span>","more")}
is not a valid expression so it has to be purified.
Whereas
{if(1 < 2,"less","more")}
is a valid expression and has no need to be purified.

In other words: a valid expression can't contain a XSS, can it? So there is no need to purify it.

I guess a question may contain a mixture of expressions and normal text so it gets complicated...

DenisChenu

DenisChenu

2014-03-06 17:47

developer   ~29130

Starting point :
echo preg_replace("/{(.)( > )(.)}/","{"."$1 > $3"."}",preg_replace("/{(.)( < )(.)}/","{"."$1 < $3"."}",purify($string)));

But need more control (na space, no lf after { or before }.

And need to test some update too "EM string"

{if(1 < 2," < "," > ")}
=>
{if(1 < 2," & lt ; "," & gt ; ")}

DenisChenu

DenisChenu

2014-03-07 16:50

developer   ~29141

In https://github.com/LimeSurvey/LimeSurvey/pull/173 but maybe more test to find if there are a way for user to add XSS or some other broken security.

DenisChenu

DenisChenu

2014-03-10 18:25

developer   ~29167

Last edited: 2014-03-10 18:27

Another solution :
https://github.com/LimeSurvey/LimeSurvey/pull/174

PS: {if(1 lt 2,"<span onmouseover='alert(XSS)'>mouse hover</span>","more")} IS a valid expression.

c_schmitz

c_schmitz

2014-03-17 13:14

administrator   ~29269

2.05+ Build 140317 released

Related Changesets

LimeSurvey: master 40ba010e

2014-03-07 14:33:31

DenisChenu

Details Diff
Dev: start 08818: Comparison operators (< and >) get converted to HTML-entities in expressions in questions
Dev: xss filtering is done in Model : need only one place, then remove old system
Dev: remove some $_POST update : title don't need to be filtered
Dev: Unable to save directly a question in second language survey (js selector)
Affected Issues
08818
mod - application/controllers/admin/database.php Diff File
mod - scripts/admin/questions.js Diff File

LimeSurvey: master 36ea616f

2014-03-13 10:11:29

DenisChenu

Details Diff
fixed issue 08818: XSS security enable : Comparison operators (< and >) get converted to HTML-entities in expressions in questions
Dev: Use ExpressionManager in xssFilter
Dev: need some update on EM core to leave space, tab and \n
Dev: Add a public function to tokenize any string in EM core (but why RDP_Tokenize is private ?)
Affected Issues
08818
mod - application/core/LSYii_Validators.php Diff File
mod - application/helpers/expressions/em_core_helper.php Diff File

Issue History

Date Modified Username Field Change
2014-03-06 15:25 hermann New Issue
2014-03-06 15:26 hermann Note Added: 29119
2014-03-06 15:34 c_schmitz Assigned To => DenisChenu
2014-03-06 15:34 c_schmitz Status new => assigned
2014-03-06 15:52 DenisChenu Note Added: 29120
2014-03-06 15:53 DenisChenu Note Edited: 29120
2014-03-06 15:53 Mazi Note Added: 29121
2014-03-06 16:04 DenisChenu Note Added: 29122
2014-03-06 16:09 hermann Note Added: 29123
2014-03-06 16:11 DenisChenu Note Added: 29124
2014-03-06 16:36 hermann Note Added: 29126
2014-03-06 16:37 hermann Note Edited: 29126
2014-03-06 17:47 DenisChenu Note Added: 29130
2014-03-07 15:33 DenisChenu Changeset attached => LimeSurvey master 40ba010e
2014-03-07 16:50 DenisChenu Note Added: 29141
2014-03-10 18:25 DenisChenu Note Added: 29167
2014-03-10 18:27 DenisChenu Note Edited: 29167
2014-03-13 11:58 DenisChenu Status assigned => resolved
2014-03-13 11:58 DenisChenu Fixed in Version => 2.05+
2014-03-13 11:58 DenisChenu Resolution open => fixed
2014-03-17 13:14 c_schmitz Note Added: 29269
2014-03-17 13:14 c_schmitz Status resolved => closed
2014-03-17 13:37 DenisChenu Changeset attached => LimeSurvey master 36ea616f
2014-03-21 09:07 DenisChenu Relationship added has duplicate 08880
2014-03-21 09:47 DenisChenu Relationship added related to 08887