First round of fixes of several vulnarabilities

- The vulnarabilities would enable unauthorized persons to modify data in your Xerte Toolkits installation Many thanks to Bauke Gehem, System addin at Summa College, Eindhoven, The Netherlands
thexerteproject · Mar 25, 2021 · a7f8999 · a7f8999
1 parent 55d2391
commit a7f8999
Show file tree

Hide file tree

Showing 40 changed files with 729 additions and 526 deletions.
diff --git a/feedback.php b/feedback.php
@@ -55,7 +55,8 @@ function show_peer_form($row, $retouremail)
 				<?php echo XERTE_FEEDBACK_CONTACT_INSTRUCTIONS; ?></p>
                 </div>
                 <div style="width:90% !important; display:inline-block; position: fixed;">
-                    <form name="peer" action="javascript:send_review('<?php echo $retouremail; ?>','<?php echo $row['template_id']; ?>')" method="post" enctype="text/plain">
+                    <!-- form name="peer" action="javascript:send_review('<?php echo $retouremail; ?>','<?php echo $row['template_id']; ?>')" method="post" enctype="text/plain"-->
+                    <form name="peer" action="javascript:send_review()" method="post" enctype="text/plain">
                         <textarea style="width:100%; height:40vh;font-size:14pt" name="response"></textarea>
                         <br/>
                         <button type="submit" class="xerte_button" style="margin-top:5px"><?php echo XERTE_FEEDBACK_BUTTON_SEND; ?></button>
@@ -106,6 +107,7 @@ function show_peer_form($row, $retouremail)
         $retouremail = $extra[1];
 
     $_SESSION['template_id'] = $template_id;
+    $_SESSION['retouremail'] = $retouremail;
     show_peer_form($row_play, $retouremail);
 
 }
diff --git a/languages/en-GB/modules/xerte/peer.inc b/languages/en-GB/modules/xerte/peer.inc
@@ -14,8 +14,8 @@
 	define("XERTE_PEER_DESCRIPTION", "Peer review page");
 
 	define("XERTE_PEER_GUIDANCE", "Please review this learning object. Use the feedback form below to submit your feedback.");
-	
-	define("XERTE_PEER_TEXTAREA_INSTRUCTIONS", "You have been asked to provide some feedback on this learning object. Please enter your feedback and click save when you have finished. This feedback is anonymous.");
+
+    define("XERTE_PEER_TEXTAREA_INSTRUCTIONS", "You have been asked to provide some feedback on this learning object created on the Xerte installation {url} by {creator}. Please enter your feedback and click save when you have finished. This feedback is anonymous. If you wish to be contacted please add your name and contact details to your response before clicking send. This feedback will be sent to {email}");
 
     define("XERTE_PEER_BUTTON_SEND", "Send");
 

diff --git a/languages/en-GB/peer.inc b/languages/en-GB/peer.inc
@@ -16,9 +16,9 @@
 
     define("XERTE_PEER_GUIDANCE", "Please review this learning object. Use the feedback form below to submit your feedback.");
 
-define("XERTE_PEER_LIGHTBOX", "View in lightbox");
+    define("XERTE_PEER_LIGHTBOX", "View in lightbox");
 
-    define("XERTE_PEER_TEXTAREA_INSTRUCTIONS", "You have been asked to provide some feedback on this learning object. Please enter your feedback and click send when you have finished. This feedback is anonymous. If you wish to be contacted please add your name and contact details to your response before clicking send.");
+    define("XERTE_PEER_TEXTAREA_INSTRUCTIONS", "You have been asked to provide some feedback on this learning object created on the Xerte installation {url} by {creator}. Please enter your feedback and click send when you have finished. This feedback is anonymous. If you wish to be contacted please add your name and contact details to your response before clicking send. This feedback will be sent to {email}");
 
     define("XERTE_PEER_BUTTON_SEND", "Send");
 

diff --git a/languages/en-GB/website_code/php/peer/peer_review.inc b/languages/en-GB/website_code/php/peer/peer_review.inc
@@ -21,7 +21,9 @@
 
 	define("PEER_REVIEW_EMAIL_SIGNATURE","The Xerte Project");
 
-	define("PEER_REVIEW_USER_FEEDBACK","Your feedback has been sent to the user");
+    define("PEER_REVIEW_IDENTIFICATION","This email was sent as feedback on learning object {template_id} of the Xerte installation hosted at {url}");
+
+    define("PEER_REVIEW_USER_FEEDBACK","Your feedback has been sent to the user");
 
 	define("PEER_REVIEW_PROBLEM","A problem has occured");
 
diff --git a/peer.php b/peer.php
@@ -36,6 +36,12 @@
 
 function show_peer_template_form($row, $retouremail)
 {
+    global $xerte_toolkits_site;
+    $helptext = XERTE_PEER_TEXTAREA_INSTRUCTIONS;
+    $helptext = str_replace("{creator}", $row['firstname'] . ' ' . $row['surname'], $helptext);
+    $helptext = str_replace("{url}", $xerte_toolkits_site->site_url, $helptext);
+    $helptext = str_replace("{email}", $retouremail, $helptext);
+
     ?>
 <html>
     <head>
@@ -53,8 +59,9 @@ function show_peer_template_form($row, $retouremail)
                     <p><?php echo XERTE_PEER_GUIDANCE; ?> <a href="show_peer.php" data-featherlight="iframe" data-featherlight-iframe-style="display:block;border:none;height:85vh;width:85vw;"><?php echo XERTE_PEER_LIGHTBOX; ?></a></p>
                 </div>
                 <div style="width:24%; display:inline-block; position: fixed;">
-                    <form name="peer" action="javascript:send_review('<?php echo $retouremail; ?>','<?php echo $row['template_id']; ?>')" method="post" enctype="text/plain">
-                        <textarea style="width:100%; height:70vh;" name="response"><?php echo XERTE_PEER_TEXTAREA_INSTRUCTIONS; ?></textarea>
+                    <!--form name="peer" action="javascript:send_review('<?php //echo $retouremail; ?>','<?php //echo $row['template_id']; ?>')" method="post" enctype="text/plain"-->
+                    <form name="peer" action="javascript:send_review()" method="post" enctype="text/plain">
+                        <textarea style="width:100%; height:70vh;" name="response"><?php echo $helptext; ?></textarea>
                         <br/>
                         <button type="submit" class="xerte_button"><?php echo XERTE_PEER_BUTTON_SEND; ?></button>
                     </form>
@@ -105,7 +112,7 @@ function show_peer_login_form($mesg="")
 
 if(!empty($query_for_peer_response)) {
 
-    $query_for_play_content = "select otd.template_name, otd.parent_template, ld.username, otd.template_framework, tr.user_id, tr.folder, tr.template_id, td.access_to_whom, td.extra_flags";
+    $query_for_play_content = "select otd.template_name, otd.parent_template, ld.username, ld.surname, ld.firstname, otd.template_framework, tr.user_id, tr.folder, tr.template_id, td.access_to_whom, td.extra_flags";
     $query_for_play_content .= " from " . $xerte_toolkits_site->database_table_prefix . "originaltemplatesdetails otd, " . $xerte_toolkits_site->database_table_prefix . "templaterights tr, " . $xerte_toolkits_site->database_table_prefix . "templatedetails td, " . $xerte_toolkits_site->database_table_prefix . "logindetails ld";
     $query_for_play_content .= " where td.template_type_id = otd.template_type_id and td.creator_id = ld.login_id and tr.template_id = td.template_id and tr.template_id=" . $template_id .  " and (role='creator' or role='co-author')";
 
@@ -146,6 +153,7 @@ function show_peer_login_form($mesg="")
              *  Output the code
              */
             $_SESSION['template_id'] = $template_id;
+            $_SESSION['retouremail'] = $retouremail;
             show_peer_template_form($row_play, $retouremail);
         }else{
             show_peer_login_form(PEER_LOGON_FAIL);

diff --git a/show_peer.php b/show_peer.php
@@ -53,7 +53,7 @@
 
 if(!empty($query_for_peer_response)) {
 
-    $query_for_play_content = "select otd.template_name, otd.parent_template, ld.username, otd.template_framework, tr.user_id, tr.folder, tr.template_id, td.access_to_whom, td.extra_flags";
+    $query_for_play_content = "select otd.template_name, otd.parent_template, ld.username, otd.template_framework, tr.user_id, tr.folder, tr.template_id, td.access_to_whom, td.date_modified, td.extra_flags";
     $query_for_play_content .= " from " . $xerte_toolkits_site->database_table_prefix . "originaltemplatesdetails otd, " . $xerte_toolkits_site->database_table_prefix . "templaterights tr, " . $xerte_toolkits_site->database_table_prefix . "templatedetails td, " . $xerte_toolkits_site->database_table_prefix . "logindetails ld";
     $query_for_play_content .= " where td.template_type_id = otd.template_type_id and td.creator_id = ld.login_id and tr.template_id = td.template_id and tr.template_id=" . $template_id .  " and (role='creator' or role='co-author')";
 

diff --git a/website_code/php/folder_library.php b/website_code/php/folder_library.php
@@ -196,3 +196,15 @@ function move_folder($folder_id,$destination)
     }
 }
 
+function has_rights_to_this_folder($folder_id, $user_id){
+    global $xerte_toolkits_site;
+    $query = "select * from {$xerte_toolkits_site->database_table_prefix}folderdetails where login_id=? AND folder_id = ?";
+    $result = db_query_one($query, array($user_id, $folder_id));
+
+    if(!empty($result)) {
+        return true;
+    }
+    return false;
+}
+
+
diff --git a/website_code/php/folderproperties/folder_content_template.php b/website_code/php/folderproperties/folder_content_template.php
@@ -35,6 +35,12 @@
 
 include "../display_library.php";
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
+
 /**
  * connect to the database
  */

diff --git a/website_code/php/folderproperties/folder_rss_template.php b/website_code/php/folderproperties/folder_rss_template.php
@@ -33,6 +33,12 @@
 
 include "../url_library.php";
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
+
 //connect to the database
 
 $parameters = explode("_", $_POST['folder_id']);

diff --git a/website_code/php/folderproperties/folderproperties_template.php b/website_code/php/folderproperties/folderproperties_template.php
@@ -33,6 +33,12 @@
 
 include "../url_library.php";
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
+
 //connect to the database
 
 if(is_numeric($_POST['folder_id'])){

diff --git a/website_code/php/folderproperties/rename_folder_template.php b/website_code/php/folderproperties/rename_folder_template.php
@@ -32,6 +32,11 @@
 _load_language_file("/website_code/php/folderproperties/folderproperties_template.inc");
 _load_language_file("/website_code/php/folderproperties/rename_folder_template.inc");
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
 
 if(is_numeric($_POST['folder_id'])&&is_string($_POST['folder_name'])){
 

diff --git a/website_code/php/folders/copy_to_new_folder.php b/website_code/php/folders/copy_to_new_folder.php
@@ -30,6 +30,12 @@
 require_once('../../../config.php');
 include '../folder_library.php';
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
+
 if (isset($_POST['folder_id']))
 {
     move_folder($_POST['folder_id'], $_POST['destination']);

diff --git a/website_code/php/folders/delete_folder.php b/website_code/php/folders/delete_folder.php
@@ -30,6 +30,12 @@
 require_once('../../../config.php');
 include "../folder_library.php";
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
+
 delete_folder($_POST['folder_id']);
 
 ?>
diff --git a/website_code/php/folders/make_new_folder.php b/website_code/php/folders/make_new_folder.php
@@ -30,4 +30,10 @@
 require_once("../../../config.php");
 include '../folder_library.php';
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
+
 make_new_folder($_POST['folder_id'],$_POST['folder_name']);
diff --git a/website_code/php/peer/peer_review.php b/website_code/php/peer/peer_review.php
@@ -30,22 +30,22 @@
 
 _load_language_file("/website_code/php/peer/peer_review.inc");
 
-if(empty($_POST['template_id'])) {
+if(empty($_SESSION['template_id'])) {
     die("invalid form submission");
 }
 
 $query_for_file_name = "select template_name from {$xerte_toolkits_site->database_table_prefix}templatedetails where template_id =?";
 
-$row_template_name = db_query_one($query_for_file_name, array($_POST['template_id']));
+$row_template_name = db_query_one($query_for_file_name, array($_SESSION['template_id']));
 
 $query_for_access_to_whom = "select access_to_whom from {$xerte_toolkits_site->database_table_prefix}templatedetails where template_id =?";
 
-$row_access_to_whom = db_query_one($query_for_access_to_whom, array($_POST['template_id']));
+$row_access_to_whom = db_query_one($query_for_access_to_whom, array($_SESSION['template_id']));
 $access=$row_access_to_whom["access_to_whom"];
 
 $headers = get_email_headers();
 
-if(isset($_POST['retouremail'])){
+if(isset($_SESSION['retouremail'])){
 
     if($xerte_toolkits_site->apache=="true") {
         $playstring = "peerreview_";
@@ -59,12 +59,15 @@
         }
     }
 
+    $identification = PEER_REVIEW_IDENTIFICATION;
+    $identification = str_replace("{template_id}", $_SESSION['template_id'], $identification);
+    $identification = str_replace("{url}", $xerte_toolkits_site->site_url, $identification);
 
     $subject = PEER_REVIEW_FEEDBACK . " - \"" . str_replace("_"," ",$row_template_name['template_name']) ."\"";
 
-    $message = PEER_REVIEW_EMAIL_GREETING . " <br><br> " . PEER_REVIEW_EMAIL_INTRO . " ". str_replace("_"," ",$row_template_name['template_name']) ."."."<br><br><br><a href='" . $xerte_toolkits_site->site_url . $playstring . $_POST['template_id'] . "'>" . $xerte_toolkits_site->site_url . $playstring . $_POST['template_id'] . "</a><br><br><br>" . str_replace("\n", "<br>\n", $_POST['feedback']) . "<br><br><br>" . PEER_REVIEW_EMAIL_YOURS . "<br><br>" . PEER_REVIEW_EMAIL_SIGNATURE;
+    $message = PEER_REVIEW_EMAIL_GREETING . " <br><br> " . PEER_REVIEW_EMAIL_INTRO . " ". str_replace("_"," ",$row_template_name['template_name']) ."."."<br><br><br><a href='" . $xerte_toolkits_site->site_url . $playstring . $_SESSION['template_id'] . "'>" . $xerte_toolkits_site->site_url . $playstring . $_SESSION['template_id'] . "</a><br><br><br>" . str_replace("\n", "<br>\n", $_POST['feedback']) . "<br><br><br>" . PEER_REVIEW_EMAIL_YOURS . "<br><br>" . PEER_REVIEW_EMAIL_SIGNATURE . "<br><br>" . $identification;
 
-    if(mail( $_POST['retouremail'], $subject, $message, $headers)){
+    if(mail( $_SESSION['retouremail'], $subject, $message, $headers)){
 
         echo "<b>" . PEER_REVIEW_USER_FEEDBACK . "</b>";
 

diff --git a/website_code/php/properties/access_change_template.php b/website_code/php/properties/access_change_template.php
@@ -34,6 +34,12 @@
 
 include "properties_library.php";
 
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
+}
+
 /**
  * 
  * Function template share status
@@ -64,21 +70,22 @@ function template_share_status($string){
  * Update the database setting
  */
 $prefix = $xerte_toolkits_site->database_table_prefix;
+if(is_user_creator_or_coauthor($_POST['template_id'])||is_user_admin()) {
+    $query = "UPDATE {$prefix}templatedetails SET access_to_whom = ? WHERE template_id = ?";
+    if (isset($_POST['server_string'])) {
+        $access_to_whom = $_POST['access'] . '-' . $_POST['server_string'];
+    } else {
+        $access_to_whom = $_POST['access'];
+    }
 
- $query = "UPDATE {$prefix}templatedetails SET access_to_whom = ? WHERE template_id = ?";
-if(isset($_POST['server_string'])){
-    $access_to_whom = $_POST['access'] . '-' . $_POST['server_string'];    
-}else{
-    $access_to_whom = $_POST['access'];
-}
-
-$params = array($access_to_whom, $_POST['template_id']);
-$ok = db_query($query, $params);
+    $params = array($access_to_whom, $_POST['template_id']);
+    $ok = db_query($query, $params);
 
-if($ok === false) {
-    access_display_fail();
+    if ($ok === false) {
+        access_display_fail();
 
-}else {
+    } else {
 
-    access_display($xerte_toolkits_site, true);
+        access_display($xerte_toolkits_site, true);
+    }
 }
diff --git a/website_code/php/properties/delete_file_template.php b/website_code/php/properties/delete_file_template.php
@@ -31,12 +31,11 @@
 include "../error_library.php";
 include "../../../config.php";
 
-/** XXX/ TODO SECURITY HOLE - NEED TO CHECK $_POST['file'] IS VALID */
-
-if(!isset($_SESSION['toolkits_logon_username'])) {
-    die("Sorry; you can't delete this without being logged in.");
+if (!isset($_SESSION['toolkits_logon_username']))
+{
+    _debug("Session is invalid or expired");
+    die("Session is invalid or expired");
 }
-
 if(unlink(urldecode($_POST['file']))){
     receive_message($_SESSION['toolkits_logon_username'], "FILE", "SUCCESS", "The file " . $_POST['file'] . "has been deleted", "User " . $_SESSION['toolkits_logon_username'] . " has deleted " . $_POST['file']);
 }else{