From 325f4f558b493f5752fa808802d2e75b9e441a51 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Thu, 27 Nov 2014 00:36:43 +0100 Subject: [PATCH] Fixed issue #09260: XSS in browse response Fixed issue : awfull screen for browse responses Dev : default permission search is read (just to do : Permission::model()->hasGlobalPermission('superadmin') :) ) Dev : header and footer of jqgris is allways shown completely on screen (if you don't resize the window ?) Dev : Use .tooltip from jquery-ui in jqgrid table : todo : test with more column ? tested with big example file --- application/controllers/admin/responses.php | 108 ++++++++++++-------- application/helpers/common_helper.php | 1 + application/models/Permission.php | 12 +-- scripts/admin/listresponse.js | 81 +++++++++------ styles/adminstyle.css | 21 ++-- 5 files changed, 140 insertions(+), 83 deletions(-) diff --git a/application/controllers/admin/responses.php b/application/controllers/admin/responses.php index 2c3e7455cc5..0cf78dfc742 100644 --- a/application/controllers/admin/responses.php +++ b/application/controllers/admin/responses.php @@ -323,6 +323,15 @@ public function index($iSurveyID) function browse($iSurveyID) { + if(!Permission::model()->hasSurveyPermission($iSurveyID,'responses','read')) + { + $aData['surveyid'] = $iSurveyID; + $message['title']= gT('Access denied!'); + $message['message']= gT('You do not have sufficient rights to access this page.'); + $message['class']= "error"; + $this->_renderWrappedTemplate('survey', array("message"=>$message), $aData); + Yii::app()->end(); + } App()->getClientScript()->registerPackage('jqgrid'); App()->getClientScript()->registerScriptFile(Yii::app()->getConfig('adminscripts') . "listresponse.js"); @@ -342,20 +351,17 @@ function browse($iSurveyID) //add token to top of list if survey is not private if ($aData['surveyinfo']['anonymized'] == "N" && tableExists('tokens_' . $iSurveyID)) //add token to top of list if survey is not private { - $column_model[] = array('name' => 'Token', 'model_name' =>'token', 'index' => 'token', 'sorttype' => 'string', 'sortable' => false, 'width' => '100', 'align' => 'left', 'editable' => false); - $column_model[] = array('name' => 'First name', 'model_name' =>'firstname', 'index' => 'firstname', 'sorttype' => 'string', 'sortable' => true, 'width' => '100', 'align' => 'left', 'editable' => false); - $column_model[] = array('name' => 'Last Name', 'model_name' =>'lastname', 'index' => 'lastname', 'sorttype' => 'string', 'sortable' => true, 'width' => '100', 'align' => 'left', 'editable' => false); - $column_model[] = array('name' => 'Email', 'model_name' =>'email', 'index' => 'email', 'sorttype' => 'string', 'sortable' => true, 'width' => '100', 'align' => 'left', 'editable' => false); + $column_model[] = array('name'=>'token', 'model_name'=>'Token', 'index'=>'token', 'sorttype'=>'string', 'sortable'=>true, 'width'=>'100', 'align'=>'left', 'editable'=>false); + $column_model[] = array('name'=>'firstname','model_name'=>gt('First name'), 'index'=>'firstname', 'sorttype'=>'string', 'sortable'=>true, 'width'=>'100', 'align'=>'left', 'editable'=>false); + $column_model[] = array('name'=>'lastname', 'model_name'=>gt('Last Name'), 'index'=>'lastname', 'sorttype'=>'string', 'sortable'=>true, 'width'=>'100', 'align'=>'left', 'editable'=>false); + $column_model[] = array('name'=>'email', 'model_name'=>gt('Email'), 'index'=>'email', 'sorttype'=>'string', 'sortable'=>true, 'width'=>'100', 'align'=>'left', 'editable'=>false); } - $column_model[] = array('name' => 'completed', 'model_name' => 'Completed', 'index' => 'completed', 'sorttype' => 'string', 'sortable' => true, 'width' => '100', 'align' => 'left', 'editable' => false); + $column_model[] = array('name' => 'completed', 'model_name'=>gt('Completed'), 'index'=>'completed', 'sorttype'=>'string', 'sortable'=>true, 'width'=>'100', 'align'=>'left', 'editable'=> false); - - // $fields = createFieldMap($iSurveyID, 'full', false, false, $aData['language']); $fields = createFieldMap($iSurveyID,'full', true, false, $aData['language']); - - - // foreach ($fields as $fielddetails) + // AN array to control unicity of $code (EM code) + $aCodes=array(); foreach ($fields as $fielddetails) { @@ -364,9 +370,7 @@ function browse($iSurveyID) if ($fielddetails['fieldname'] == 'lastpage' || $fielddetails['fieldname'] == 'submitdate') continue; - // //////////////////////////////////////////////////////////////////////////////////////////////////////////// // Issue_9207 - Excluded token to prevent it from being included twice in the table. - // //////////////////////////////////////////////////////////////////////////////////////////////////////////// if ($fielddetails['fieldname'] == 'token') continue; @@ -415,40 +419,50 @@ function browse($iSurveyID) $fnames[] = array($fielddetails['fieldname'], "File count"); } */ - - // if ( ($fielddetails['fieldname'] == 'id') || ($fielddetails['fieldname'] == 'startlanguage') ) { - // Combine title and aid to provide a unique column header. - if ( empty($fielddetails['title']) ) { - $fielddetails['title'] = $fielddetails['fieldname']; + // TODO: upload question type have more than one column (see before) + // Construction of clean name and title + $code=viewHelper::getFieldCode($fielddetails,array('LEMcompat'=>true));// This must be unique ...... + //fix unicity of $code + if(isset($aCodes[$code])) + { + $aCodes[$code]++; + $code="{$code}-{$aCodes[$code]}"; } - if ( !empty($fielddetails->aid) ) { - $fielddetails['title'] = $fielddetails['title'] . '_' . $fielddetails->aid; + else + { + $aCodes[$code]=0; } - - $fnames[] = array($fielddetails['fieldname'], $fielddetails['title']); - - $column_model[] = array('name' => $fielddetails['title'], 'model_name' => strip_tags(FlattenText(substr($fielddetails['title'], 0, 32), true)), 'index' => $fielddetails['title'], 'sorttype' => 'string', 'sortable' => true, 'width' => '100', 'align' => 'left', 'editable' => false, 'title' => strip_tags(FlattenText($fielddetails['title'])) ); - + $text=viewHelper::getFieldText($fielddetails); + $textabb=viewHelper::getFieldText($fielddetails,array('abbreviated'=>10)); + $column_model[] = array( + 'name' => $code,// Must be unique : it's true only for new survey + 'model_name' => $textabb, + 'index' => $fielddetails['fieldname'], + 'sorttype' => 'string', + 'sortable' => true, + 'width' => '100', + 'align' => 'left', + 'editable' => false, + 'title' => $text, + ); } - $column_model_txt = ls_json_encode($column_model); + $column_model_txt = ls_json_encode($column_model); $column_names = array(); - foreach ($column_model as $column) { - // $column_name = stripTagsFull($column['model_name']); - // $column_name = substr($column['model_name'], 0, 32); - // $column_names[] = FlattenText($column['model_name'],true) ; - $column_names[] = $column['model_name']; + foreach ($column_model as $column) + { + $column_names[] = "{$column['name']} {$column['model_name']}"; } - - $column_names_txt = ls_json_encode($column_names); + $column_names_txt = ls_json_encode($column_names); Yii::app()->loadHelper('surveytranslator'); $aData['issuperadmin'] = false; - if (Yii::app()->session['USER_RIGHT_SUPERADMIN'] == 1) + if (Permission::model()->hasGlobalPermission('superadmin')) { $aData['issuperadmin'] = true; + tracevar('OK'); } $aData['surveyid'] = $iSurveyID; $aData['column_model_txt'] = $column_model_txt; @@ -467,7 +481,10 @@ function browse($iSurveyID) */ public function getResponses_json($iSurveyID) { - + if(!Permission::model()->hasSurveyPermission($iSurveyID,'responses','read')) + { + Yii::app()->end(); + } $aData = $this->_getData($iSurveyID); extract($aData); @@ -476,7 +493,6 @@ public function getResponses_json($iSurveyID) $sImageURL = Yii::app()->getConfig('adminimageurl'); - $fnames = array(); //add token to top of list if survey is not private if ($aData['surveyinfo']['anonymized'] == "N" && tableExists('tokens_' . $iSurveyID)) //add token to top of list if survey is not private @@ -537,6 +553,10 @@ public function getResponses_json($iSurveyID) // issue_9207 - added join of survey table with token table. // //////////////////////////////////////////////////////////////////////////////////////////////////////////// + $sOrder=Yii::app()->request->getParam('order') == 'desc' ? 'desc' : 'asc';// ajax use sort + $iLimit=Yii::app()->request->getParam('limit'); // We need to set a maximum : else memory issue + $iStart=Yii::app()->request->getParam('start'); + // Old behaviour : ajax default request from jqgrid need sort / rows (limit) / page (start) / sidx for order by : use javacript log please .... $oCriteria = new CDbCriteria; //Create the query if ($aData['surveyinfo']['anonymized'] == "N" && tableExists("{{tokens_{$iSurveyID}}}") && Permission::model()->hasSurveyPermission($iSurveyID,'tokens','read')) @@ -555,20 +575,23 @@ public function getResponses_json($iSurveyID) $dtcount = SurveyDynamic::model($iSurveyID)->count($oCriteria);// or die("Couldn't get response data
"); - if ($limit > $dtcount) + // limit or row ? + if (!$iLimit || $limit > $dtcount) { - $limit = $dtcount; + $iLimit = $dtcount; } //NOW LETS SHOW THE DATA if (Yii::app()->request->getPost('sql') && stripcslashes(Yii::app()->request->getPost('sql')) !== "" && Yii::app()->request->getPost('sql') != "NULL") $oCriteria->addCondition(stripcslashes(Yii::app()->request->getPost('sql'))); - if (!is_null($tokenRequest)) { + if (!is_null(Yii::app()->request->getParam('token'))) { $oCriteria->addCondition('t.token = ' . Yii::app()->db->quoteValue($tokenRequest)); } - $oCriteria->order = 'id ' . ($order == 'desc' ? 'desc' : 'asc'); + $oCriteria->order = "id {$sOrder}"; + // We don't use ajax then : we never update this ! + // And we need to return the number of complete row and number of page $oCriteria->offset = (Yii::app()->request->getPost('page', 1) - 1) * 50; $oCriteria->limit = Yii::app()->request->getPost('rows', 50); @@ -642,8 +665,8 @@ public function getResponses_json($iSurveyID) ))) { continue; } - - $aSurveyEntry[] = $row_value; + // Alternative to striptag : use CHtmlPurifier : but CHtmlPurifier use a lot of memory + $aSurveyEntry[] = strip_tags(getExtendedAnswer($iSurveyID, $row_index, $row_value, $oBrowseLanguage)); // This fix XSS and get the value } @@ -653,7 +676,8 @@ public function getResponses_json($iSurveyID) } $aSurveyEntries->rows = $all_rows; - + //viewHelper::disableHtmlLogging(); It's better with but we need to fix error actually + header('Content-type: application/json'); echo ls_json_encode($aSurveyEntries); } diff --git a/application/helpers/common_helper.php b/application/helpers/common_helper.php index 80fed737807..dbbbeff55eb 100644 --- a/application/helpers/common_helper.php +++ b/application/helpers/common_helper.php @@ -5316,6 +5316,7 @@ function getAttributeValue($surveyid,$attrName,$token) */ function stripJavaScript($sContent){ $text = preg_replace('@]*?>.*?@si', '', $sContent); + // TODO : Adding the onload/onhover etc ... or remove this false security function return $text; } diff --git a/application/models/Permission.php b/application/models/Permission.php index 514b7e8f6c4..6ac541dd0f1 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -472,7 +472,7 @@ function copySurveyPermissions($iSurveyIDSource,$iSurveyIDTarget) * @param $iUserID integer User ID - if not given the one of the current user is used * @return bool True if user has the permission */ - function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD, $iUserID=null) + function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD='read', $iUserID=null) { static $aPermissionStatic; @@ -546,11 +546,11 @@ function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD, $iUserID= * @param $iUserID integer User ID - if not given the one of the current user is used * @return bool True if user has the permission */ - function hasGlobalPermission($sPermission, $sCRUD, $iUserID=null) + function hasGlobalPermission($sPermission, $sCRUD='read', $iUserID=null) { return $this->hasPermission(0, 'global', $sPermission, $sCRUD, $iUserID); - } - + } + /** * Checks if a user has a certain permission in the given survey * @@ -560,10 +560,10 @@ function hasGlobalPermission($sPermission, $sCRUD, $iUserID=null) * @param $iUserID integer User ID - if not given the one of the current user is used * @return bool True if user has the permission */ - function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD, $iUserID=null) + function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD='read', $iUserID=null) { return $this->hasPermission($iSurveyID, 'survey', $sPermission, $sCRUD, $iUserID); - } + } /** * Returns true if a user has permission to use a certain template diff --git a/scripts/admin/listresponse.js b/scripts/admin/listresponse.js index b2a4f45e8ba..e85d1cf962e 100755 --- a/scripts/admin/listresponse.js +++ b/scripts/admin/listresponse.js @@ -1,6 +1,19 @@ //$Id: listsurvey.js 9692 2012-12-10 21:31:10Z pradesh $ //V1.1 Pradesh - Copied from listsurvey.js +/* +* Scroll the pager and the footer when scrolling horizontally +* Maybe for token table too +*/ +$(window).scroll(function(){ + $('.ui-jqgrid-toppager').css({ + 'left': $(this).scrollLeft() + }); + $('.ui-jqgrid-pager').css({ + 'left': $(this).scrollLeft() + }); +}); + $(document) .ready( function() { @@ -77,7 +90,7 @@ $(document) var field; $('#searchbutton').click(function() { - + // Must be done }); var lastSel, lastSel2; function returnColModel() { @@ -99,7 +112,7 @@ $(document) url : jsonUrl, // editurl : editUrl, datatype : "json", - mtype : "post", + mtype : "POST", colNames : colNames, colModel : returnColModel(), toppager : true, @@ -107,18 +120,30 @@ $(document) shrinkToFit : false, ignoreCase : true, rowNum : 25, - editable : true, + editable : false, scrollOffset : 0, sortable : true, hidegrid : false, - sortname : 'sid', + sortname : 'id', sortorder : 'asc', viewrecords : true, rowList : [ 25, 50, 100, 250, 500, 1000 ], multiselect : true, - loadonce : true, + loadonce : true, // loadonce : false, to use ajax request (else memory issue) pager : "#pager", - caption : sCaption + caption : sCaption, + beforeRequest: function(){ + // ACtuvate tooltip on header + for (i = 0; i < colModels.length; i++) { + var col=i+1; + $("tr.ui-jqgrid-labels th:eq("+col+") .questiontext").attr('title',colModels[i]['title']); + } + $(".ui-jqgrid-labels").tooltip(); + }, + loadComplete: function(){ + // actvate tooltip on answers + $("#displayresponses").tooltip({ tooltipClass: "tooltip-text" }); + } }); jQuery("#displayresponses").jqGrid( 'navGrid', @@ -213,19 +238,16 @@ $(document) minHeight : 100 }); - $('.wrapper').width($('#displayresponses').width() * 1.006); - $('.footer').width( - ($('#displayresponses').width() * 1.006) - 10); +// This is really AWFULL ! +// $('.wrapper').width($('#displayresponses').width() * 1.006); +// $('.footer').width( +// ($('#displayresponses').width() * 1.006) - 10); /* Trigger the inline search when the access list changes */ - $('#gs_completed_select').change( - function() { - $("#gs_completed").val( - $('#gs_completed_select').val()); - - var e = jQuery.Event("keydown"); - $("#gs_completed").trigger(e); - }); + $(document).on('change','#gs_completed_select',function() { + $("#gs_completed").val($('#gs_completed_select').val()); + $("#gs_completed").trigger("keydown"); + }); /* Change the text search above "Status" icons to a dropdown */ var parentDiv = $('#gs_completed').parent(); @@ -239,19 +261,20 @@ $(document) $('#gs_no_filter').css("display", ""); $('#gs_Actions').css("display", "none"); - var setTooltipsOnColumnHeader = function(grid, iColumn, - text) { - var col = iColumn + 1; - var thd = jQuery("thead:first", grid[0].grid.hDiv)[0]; - jQuery("tr.ui-jqgrid-labels th:eq(" + col + ")", thd) - .attr("title", text); - }; +// It don't work, and jquery ui have own tooltip (bootstrap too) then use it +// var setTooltipsOnColumnHeader = function(grid, iColumn, +// text) { +// var col = iColumn + 1; +// var thd = jQuery("thead:first", grid[0].grid.hDiv)[0]; +// jQuery("tr.ui-jqgrid-labels th:eq(" + col + ")", thd) +// .attr("title", text); +// }; - var colmodel_count = colModels.length; - for (i = 0; i < colmodel_count; i++) { - setTooltipsOnColumnHeader($("#displayresponses"), i, - colModels[i]['title']); +// var colmodel_count = colModels.length; +// for (i = 0; i < colmodel_count; i++) { +// setTooltipsOnColumnHeader($("#displayresponses"), i, +// colModels[i]['title']); - } +// } }); diff --git a/styles/adminstyle.css b/styles/adminstyle.css index 303d0253e9a..4e08d9e6bb6 100644 --- a/styles/adminstyle.css +++ b/styles/adminstyle.css @@ -161,19 +161,28 @@ div.ui-jqgrid-hbox input, div.ui-jqgrid-hbox input{ height: auto; padding: 0 2px; } + /* Fix height of browsettable */ -.browsetable th .qcode { +#gview_displayresponses th .qcode { display: block; + padding-right:16px; /* leave place for ordering arrow */ height: auto; } -.browsetable th .questiontext { +#gbox_displayresponses.ui-jqgrid .s-ico{ + position:absolute;right:18px;top:0; /* The ordering arrow */ +} +#gview_displayresponses th .questiontext { display: block; - height: 3em; + height: 2.5em; overflow: hidden; } -.browsetable td span { - min-width:10em;/* This is OK for date time */ +#gbox_displayresponses.ui-jqgrid .ui-jqgrid-htable th div { + height: auto; } +/* jqgrid table must take all place if avalaible, but header and footer must not be outside the screen */ +/* Myabe use a pseudo fixd-y solution is a good idea */ +.ui-jqgrid,.ui-jqgrid-view,.ui-jqgrid-titlebar,.ui-jqgrid-toppager,.ui-jqgrid-pager{min-width:100%;max-width:100%;border-right-width:0;border-left-width:0} + .ui-tooltip.tooltip-text{min-width:15em} .templateeditor { float:left; @@ -230,4 +239,4 @@ li input+label { display: inline;} text-align:center; font-weight:bold; } -#quotalist form{display:inline;} \ No newline at end of file +#quotalist form{display:inline;}