Skip to content

Commit

Permalink
Fixed issue: [security] #15204: Reflected XSS vulnerabilities - thank…
Browse files Browse the repository at this point in the history
…s to J. Greil from the SEC Consult Vulnerability Lab

Dev: disable pseudo params for same param to be different : throw a 403
  • Loading branch information
Shnoulle committed Sep 1, 2019
1 parent 68e3ed2 commit f1c1ad2
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions application/core/Survey_Common_Action.php
Expand Up @@ -106,8 +106,7 @@ private function _addPseudoParams($params)
'id' => 'iId',
'gid' => 'iGroupId',
'qid' => 'iQuestionId',
/* Unsure we set 'iSurveyId', 'iSurveyID','surveyid' to same final survey id */
/* priority is surveyid,surveyId,sid : surveyId=1&sid=2 set sid surveyid to 1 */
/* priority is surveyid,surveyId,sid : surveyId=1&sid=2 set iSurveyId to 1 */
'sid' => array('iSurveyId', 'iSurveyID', 'surveyid'), // Old link use sid
'surveyId' => array('iSurveyId', 'iSurveyID', 'surveyid'), // PluginHelper->sidebody : if disable surveyId usage : broke API
'surveyid' => array('iSurveyId', 'iSurveyID', 'surveyid'),
Expand All @@ -128,13 +127,16 @@ private function _addPseudoParams($params)
// Foreach pseudo, take the key, if it exists,
// Populate the values (taken as an array) as keys in params
// with that key's value in the params
// (only if that place is empty)
// Chek is 2 params are equal for security issue.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Sep 17, 2019

Contributor

"Check if"?

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Sep 17, 2019

Author Collaborator

Yep, but code (and coment) are updated :

foreach ($pseudos as $key => $pseudo) {
if (isset($params[$key])) {
$pseudo = (array) $pseudo;
foreach ($pseudo as $pseud) {
if (empty($params[$pseud])) {
$params[$pseud] = $params[$key];
} elseif($params[$pseud] != $params[$key]){
// Throw error about multiple params (and if they are different) #15204
throw new CHttpException(403, sprintf(gT("Invalid parameter %s (%s already set)"),$pseud,$key));
}
}
}
Expand Down Expand Up @@ -286,7 +288,6 @@ private function renderCentralContents($sAction, $aViewUrls, $aData = [])
// Output
case 'output' :
//// TODO : http://goo.gl/ABl5t5

$content .= $viewUrl;

if (isset($aViewUrls['afteroutput'])) {
Expand Down

0 comments on commit f1c1ad2

Please sign in to comment.