View Issue Details

This bug affects 1 person(s).
 262
IDProjectCategoryView StatusLast Update
06312Bug reportsSecuritypublic2012-07-26 09:50
Reporteruser20647Assigned Toc_schmitz  
PrioritynormalSeveritypartial_block 
Status closedResolutionfixed 
Product Version1.92+ 
Fixed in Version1.92+ 
Summary06312: Editing Survey Security Permissions as non-super-admin and non-survey owner throws PHP Fatal Error
Description

If you try to edit a user's survey permissions as a user who is not a super-admin or owner of the survey (but does have full survey permission editing rights) LimeSurvey produces " PHP Fatal error: Cannot redeclare replacenewline() (previously declared in /html.php:2120) in html.php on line 2131"

This seems to be a problem with admin/html.php under the survey permissions section. For example there's a line ("$query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} AND owner_id = ".$_SESSION['loginID']." AND owner_id != ".$postuserid;") in html.php. Specifically the " AND owner_id = ".$_SESSION['loginID']." " seems to be the problem.

Steps To Reproduce

Create a test survey and give a test user without general admin privileges the survey security permission as well as permissions to create users. Log in as the test user. Try to add a test user and then set privileges for that user on the test survey.

TagsNo tags attached.
Attached Files
ls-patch-064312.diff (2,561 bytes)   
diff --git admin/html.php admin/html.php
index 4350fd2..fc19f93 100644
--- admin/html.php
+++ admin/html.php
@@ -1474,7 +1474,7 @@ if($action == "addsurveysecurity")
     $addsummary = "<div class='header ui-widget-header'>".$clang->gT("Add User")."</div>\n";
     $addsummary .= "<div class=\"messagebox ui-corner-all\">\n";
 
-    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} AND owner_id = ".$_SESSION['loginID']." AND owner_id != ".$postuserid;
+    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} AND owner_id != ".$postuserid;
     $result = db_execute_assoc($query); //Checked
     if( ($result->RecordCount() > 0 && in_array($postuserid,getuserlist('onlyuidarray'))) ||
     $_SESSION['USER_RIGHT_SUPERADMIN'] == 1)
@@ -1522,7 +1522,7 @@ if($action == "addusergroupsurveysecurity")
     $addsummary = "<div class=\"header\">".$clang->gT("Add user group")."</div>\n";
     $addsummary .= "<div class=\"messagebox ui-corner-all\" >\n";
 
-    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} AND owner_id = ".$_SESSION['loginID'];
+    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid}";
     $result = db_execute_assoc($query); //Checked
     if( ($result->RecordCount() > 0 && in_array($postusergroupid,getsurveyusergrouplist('simpleugidarray'))) || $_SESSION['USER_RIGHT_SUPERADMIN'] == 1)
     {
@@ -1582,7 +1582,7 @@ if($action == "delsurveysecurity")
     $addsummary = "<div class=\"header\">".$clang->gT("Deleting User")."</div>\n";
     $addsummary .= "<div class=\"messagebox\">\n";
 
-    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} AND owner_id = ".$_SESSION['loginID']." AND owner_id != ".$postuserid;
+    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} AND owner_id != ".$postuserid;
     $result = db_execute_assoc($query); //Checked
     if($result->RecordCount() > 0 || $_SESSION['USER_RIGHT_SUPERADMIN'] == 1)
     {
@@ -1609,7 +1609,7 @@ if($action == "delsurveysecurity")
 
 if($action == "setsurveysecurity" || $action == "setusergroupsurveysecurity")
 {
-    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} AND owner_id = ".$_SESSION['loginID'];
+    $query = "SELECT sid, owner_id FROM ".db_table_name('surveys')." WHERE sid = {$surveyid} ";
     if ($action == "setsurveysecurity")
     {
       $query.=  " AND owner_id != ".$postuserid;
ls-patch-064312.diff (2,561 bytes)   
Bug heat262
Complete LimeSurvey version number (& build)120623
I will donate to the project if issue is resolvedNo
Browser
Database type & versionMySQL 5.5
Server OS (if known)Centos 5
Webserver software & version (if known)Apache 2.2.3-43
PHP Version5.3.8

Relationships

has duplicate 06302 closedc_schmitz Allowed memory size of 52428800 bytes exhausted (tried to allocate 55556 bytes) in classes/expressions/LimeExpressionManager.php 

Users monitoring this issue

There are no users monitoring this issue.

Activities

user20647

2012-07-12 00:19

  ~19723

I made changes to the queries (diff attached) and the survey permissions editing now works.

Also in case anyone wants to test this, in order for a non-admin to add users to a particular survey you have to set "$usercontrolSameGroupPolicy=false;" in config.php.

c_schmitz

c_schmitz

2012-07-18 15:34

administrator   ~19758

I think the patch is not complete - you are just removing the restriction to only display users that are actually owned by the particular administrator.

But as you already described this only would be valid if $usercontrolSameGroupPolicy==false ?

Please submit an updated patch by gitHub pull request. Thank you!

user20647

2012-07-20 17:04

  ~19891

Thanks for the response!

After investigating it further, it looks like all logic about which users to display is already implemented under "if (action == "surveysecurity") ". As such, the queries and if-blocks under "addsurveysecurity", and the other survey security methods aren't actually necessary. I removed those and tested it with my changes and it seems to be working as intended. I also put a pull request on github for you to evaluate.

Also, I think this bug is independent of $usercontrolSameGroupPolicy==false, I was just doing it that way because it was easier to test. If a non-admin, non-survey owner, who can set survey permissions and has someone in their group who they added, the bug would come up too.

c_schmitz

c_schmitz

2012-07-23 09:26

administrator   ~19922

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&amp;id=9097

c_schmitz

c_schmitz

2012-07-23 09:26

administrator   ~19923

Thank you very much!

Related Changesets

LimeSurvey: master 1b05a2b5

2012-07-18 09:12:02

user20647

Details Diff
Fixed issue 06312: Editing Survey Security Permissions as non-super-admin and non-survey owner throws PHP Fatal Error Affected Issues
06312
mod - admin/html.php Diff File

LimeSurvey: master 62efe55b

2012-07-20 07:43:10

user20647

Details Diff
Fixed Issue 06312 - Editing Survey Security Permissions as non-super-admin and non-survey owner throws PHP Fatal Error
deleted unecessary if-blocks underneath "addsurveysecurity", "addusergroupsurveysecurity", "delsurveysecurity", and "setsurveysecurity"
The logic under "if (action=="surveysecurity")" already took care of all permissions, so those were not needed.
Affected Issues
06312
mod - admin/html.php Diff File

LimeSurvey: master 3cc4c73a

2012-07-23 00:25:54

c_schmitz

Details Diff
Merge pull request #14 from smking3/master

Fixed issue 06312: Editing Survey Security Permissions as non-super-admin and non-survey owner throws PHP Fatal Error
Affected Issues
06312
mod - admin/html.php Diff File

Issue History

Date Modified Username Field Change
2012-07-09 23:24 user20647 New Issue
2012-07-10 16:07 user20647 Issue Monitored: user20647
2012-07-12 00:16 user20647 File Added: ls-patch-064312.diff
2012-07-12 00:19 user20647 Note Added: 19723
2012-07-18 15:34 c_schmitz Note Added: 19758
2012-07-18 15:34 c_schmitz Assigned To => c_schmitz
2012-07-18 15:34 c_schmitz Status new => feedback
2012-07-20 17:04 user20647 Note Added: 19891
2012-07-20 17:04 user20647 Status feedback => assigned
2012-07-23 09:26 user20647 Changeset attached => LimeSurvey master 1b05a2b5
2012-07-23 09:26 user20647 Changeset attached => LimeSurvey master 62efe55b
2012-07-23 09:26 c_schmitz Changeset attached => LimeSurvey master 3cc4c73a
2012-07-23 09:26 c_schmitz Note Added: 19922
2012-07-23 09:26 c_schmitz Resolution open => fixed
2012-07-23 09:26 c_schmitz Note Added: 19923
2012-07-23 09:26 c_schmitz Status assigned => resolved
2012-07-23 09:26 c_schmitz Fixed in Version => 2.00+
2012-07-26 09:49 c_schmitz Relationship added has duplicate 06302
2012-07-26 09:50 c_schmitz Status resolved => closed
2012-07-26 09:50 c_schmitz Fixed in Version 2.00+ => 1.92+