bug #4530 [security] DOM based XSS that results to a CSRF that create…

…s a ROOT account in certain conditions Signed-off-by: Marc Delisle <marc@infomarc.info>
phpmyadmin · Sep 13, 2014 · 33b39f9 · 33b39f9
1 parent 01c553a
commit 33b39f9
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 13 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,6 +1,10 @@
 phpMyAdmin - ChangeLog
 ======================
 
+4.2.8.1 (not yet released)
+- bug #4530 [security] DOM based XSS that results to a CSRF that creates a
+            ROOT account in certain conditions
+
 4.2.8.0 (2014-08-31)
 - bug #4516 Odd export behavior
 - bug #4519 Uncaught TypeError: Cannot read property 'success' of null

diff --git a/js/ajax.js b/js/ajax.js
@@ -783,9 +783,16 @@ AJAX.setUrlHash = (function (jQuery, window) {
     if (window.location.hash.substring(0, 8) == '#PMAURL-') {
         // We have a valid hash, let's redirect the user
         // to the page that it's pointing to
-        window.location = window.location.hash.substring(
-            window.location.hash.indexOf(':') + 1
-        );
+        var colon_position = window.location.hash.indexOf(':');
+        var questionmark_position = window.location.hash.indexOf('?');
+        if (colon_position != -1 && questionmark_position != -1 && colon_position < questionmark_position) {
+            var hash_url = window.location.hash.substring(colon_position + 1, questionmark_position);
+            if (PMA_gotoWhitelist.indexOf(hash_url) != -1) {
+                window.location = window.location.hash.substring(
+                    colon_position + 1
+                );
+            }
+        }
     } else {
         // We don't have a valid hash, so we'll set it up
         // when the page finishes loading

diff --git a/js/whitelist.php b/js/whitelist.php
@@ -0,0 +1,31 @@
+<?php
+/* vim: set expandtab sw=4 ts=4 sts=4: */
+/**
+ * Exporting of $goto_whitelist from PHP to Javascript
+ *
+ * @package PhpMyAdmin
+ */
+
+chdir('..');
+
+// Send correct type:
+header('Content-Type: text/javascript; charset=UTF-8');
+
+// Cache output in client - the nocache query parameter makes sure that this
+// file is reloaded when config changes
+header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT');
+
+// Avoid loading the full common.inc.php because this would add many
+// non-js-compatible stuff like DOCTYPE
+define('PMA_MINIMUM_COMMON', true);
+require_once './libraries/common.inc.php';
+// Close session early as we won't write anything there
+session_write_close();
+
+echo "var PMA_gotoWhitelist = new Array();\n";
+$i = -1;
+foreach ($GLOBALS['goto_whitelist'] as $one_whitelist) {
+    $i++;
+    echo 'PMA_gotoWhitelist[' . $i . ']="' . $one_whitelist . '";' . "\n";
+}
+?>
diff --git a/libraries/Header.class.php b/libraries/Header.class.php
@@ -144,7 +144,15 @@ public function __construct()
      */
     private function _addDefaultScripts()
     {
+        // Localised strings
+        $params = array('lang' => $GLOBALS['lang']);
+        if (isset($GLOBALS['db'])) {
+            $params['db'] = $GLOBALS['db'];
+        }
         $this->_scripts->addFile('jquery/jquery-1.8.3.min.js');
+        $this->_scripts->addFile(
+            'whitelist.php' . PMA_URL_getCommon($params), false, true
+        );
         $this->_scripts->addFile('ajax.js');
         $this->_scripts->addFile('keyhandler.js');
         $this->_scripts->addFile('jquery/jquery-ui-1.9.2.custom.min.js');
@@ -171,11 +179,6 @@ private function _addDefaultScripts()
         // Here would not be a good place to add CodeMirror because
         // the user preferences have not been merged at this point
 
-        // Localised strings
-        $params = array('lang' => $GLOBALS['lang']);
-        if (isset($GLOBALS['db'])) {
-            $params['db'] = $GLOBALS['db'];
-        }
         $this->_scripts->addFile('messages.php' . PMA_URL_getCommon($params));
         // Append the theme id to this url to invalidate
         // the cache on a theme change. Though this might be

diff --git a/libraries/Scripts.class.php b/libraries/Scripts.class.php
@@ -50,12 +50,18 @@ class PMA_Scripts
      */
     private function _includeFiles($files)
     {
+        $first_dynamic_scripts = "";
         $dynamic_scripts = "";
         $scripts = array();
         foreach ($files as $value) {
             if (strpos($value['filename'], "?") !== false) {
-                $dynamic_scripts .= "<script type='text/javascript' src='js/"
-                    . $value['filename'] . "'></script>";
+                if ($value['before_statics'] === true) {
+                    $first_dynamic_scripts .= "<script type='text/javascript' src='js/"
+                        . $value['filename'] . "'></script>";
+                } else {
+                    $dynamic_scripts .= "<script type='text/javascript' src='js/"
+                        . $value['filename'] . "'></script>";
+                }
                 continue;
             }
             $include = true;
@@ -83,7 +89,7 @@ private function _includeFiles($files)
             '<script type="text/javascript" src="%s"></script>',
             htmlspecialchars($url)
         );
-        return $static_scripts . $dynamic_scripts;
+        return $first_dynamic_scripts . $static_scripts . $dynamic_scripts;
     }
 
     /**
@@ -105,10 +111,12 @@ public function __construct()
      * @param string $filename       The name of the file to include
      * @param bool   $conditional_ie Whether to wrap the script tag in
      *                               conditional comments for IE
+     * @param bool   $before_statics Whether this dynamic script should be
+     *                               included before the static ones
      *
      * @return void
      */
-    public function addFile($filename, $conditional_ie = false)
+    public function addFile($filename, $conditional_ie = false, $before_statics = false)
     {
         $hash = md5($filename);
         if (!empty($this->_files[$hash])) {
@@ -119,7 +127,8 @@ public function addFile($filename, $conditional_ie = false)
         $this->_files[$hash] = array(
             'has_onload' => $has_onload,
             'filename' => $filename,
-            'conditional_ie' => $conditional_ie
+            'conditional_ie' => $conditional_ie,
+            'before_statics' => $before_statics
         );
     }