View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
06312 | Bug reports | Security | public | 2012-07-09 23:24 | 2012-07-26 09:50 |
Reporter | Assigned To | c_schmitz | |||
Priority | normal | Severity | partial_block | ||
Status | closed | Resolution | fixed | ||
Product Version | 1.92+ | ||||
Fixed in Version | 1.92+ | ||||
Summary | 06312: 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. | ||||
Tags | No 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; | ||||
Bug heat | 262 | ||||
Complete LimeSurvey version number (& build) | 120623 | ||||
I will donate to the project if issue is resolved | No | ||||
Browser | |||||
Database type & version | MySQL 5.5 | ||||
Server OS (if known) | Centos 5 | ||||
Webserver software & version (if known) | Apache 2.2.3-43 | ||||
PHP Version | 5.3.8 | ||||
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. |
|
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! |
|
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. |
|
Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=9097 |
|
Thank you very much! |
|
LimeSurvey: master 1b05a2b5 2012-07-18 09:12:02
|
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
|
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 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 |
Date Modified | Username | Field | Change |
---|---|---|---|
2012-07-09 23:24 |
|
New Issue | |
2012-07-10 16:07 |
|
Issue Monitored: user20647 | |
2012-07-12 00:16 |
|
File Added: ls-patch-064312.diff | |
2012-07-12 00:19 |
|
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 |
|
Note Added: 19891 | |
2012-07-20 17:04 |
|
Status | feedback => assigned |
2012-07-23 09:26 |
|
Changeset attached | => LimeSurvey master 1b05a2b5 |
2012-07-23 09:26 |
|
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+ |