IMPORTANT SECURITY FIX: this prevents bad characters being passed to help.php

help.php

@@ -14,8 +14,8 @@
 
   print_header();
 
-  if (ereg("\\.\\.", $file)) {
-      error("Filenames can not contain \"..\"");
+  if (detect_munged_arguments("$module/$file")) {
+      error("Filenames contain illegal characters!");
   }
 
   if ($file) {
@@ -28,7 +28,7 @@
         if (file_exists("$filepath")) {
             require_once("$filepath");           // Chosen language
 
-        } else {                            // Fall back to English
+        } else {                                 // Fall back to English
             if ($module == "moodle") {
                 $filepath = "$CFG->dirroot/lang/en/help/$file";
             } else {
@@ -43,13 +43,13 @@
             }
         }
     } else {
-        echo "<P>";
+        echo "<p>";
         echo $text;
-        echo "</P>";
+        echo "</p>";
     }
 
     close_window_button();
 ?>
-</BODY>
-</HTML>
+</body>
+</html>
 

lib/weblib.php

@@ -378,6 +378,19 @@ function validate_email ($address) {
                   $address));
 }
 
+function detect_munged_arguments($string) {
+    if (ereg("\.\.", $string)) { // check for parent URLs
+        return true;
+    }
+    if (ereg("\|", $string)) {  // check for pipes
+        return true;
+    }
+    if (ereg("\'", $string)) {  // check for backquotes
+        return true;
+    }
+    return false;
+}
+
 function get_slash_arguments($file="file.php") {
 /// Searches the current environment variables for some slash arguments
 
@@ -398,16 +411,9 @@ function parse_slash_arguments($string, $i=0) {
 /// Extracts arguments from "/foo/bar/something"
 /// eg http://mysite.com/script.php/foo/bar/something
 
-    if (strpos($string, "..")) { // check for parent URLs
+    if (detect_munged_arguments($string)) {
         return false;
     }
-    if (strpos($string, "|")) {  // check for pipes
-        return false;
-    }
-    if (strpos($string, "`")) {  // check for backquotes
-        return false;
-    }
-
     $args = explode("/", $string);
 
     if ($i) {     // return just the required argument