csrf: tighten csrf code paths a little

fichtner · fichtner · commit d218b225ccaf · 2016-05-03T17:02:00.000+02:00
o restructure session open/close for better visibility
o remove potentially insecure "try again" button
o remove override for switching csrf off (defer)
diff --git a/src/www/csrf/csrf-magic.php b/src/www/csrf/csrf-magic.php
@@ -15,16 +15,6 @@
 
 // CONFIGURATION:
 
-/**
- * By default, when you include this file csrf-magic will automatically check
- * and exit if the CSRF token is invalid. This will defer executing
- * csrf_check() until you're ready.  You can also pass false as a parameter to
- * that function, in which case the function will not exit but instead return
- * a boolean false if the CSRF check failed. This allows for tighter integration
- * with your system.
- */
-$GLOBALS['csrf']['defer'] = false;
-
 /**
  * This is the amount of seconds you wish to allow before any token becomes
  * invalid; the default is two hours, which should be more than enough for
@@ -117,12 +107,6 @@
  */
 $GLOBALS['csrf']['frame-breaker'] = true;
 
-/**
- * Whether or not CSRF Magic should be allowed to start a new session in order
- * to determine the key.
- */
-$GLOBALS['csrf']['auto-session'] = true;
-
 /**
  * Whether or not csrf-magic should produce XHTML style tags.
  */
@@ -187,7 +171,6 @@ function csrf_check($fatal = true)
     if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
         return true;
     }
-    csrf_start();
     $name = $GLOBALS['csrf']['input-name'];
     $ok = false;
     $tokens = '';
@@ -232,7 +215,6 @@ function csrf_get_tokens()
     } else {
         $ip = '';
     }
-    csrf_start();
 
     // These are "strong" algorithms that don't require per se a secret
     if (session_id()) {
@@ -259,46 +241,18 @@ function csrf_get_tokens()
     return 'invalid';
 }
 
-function csrf_flattenpost($data)
-{
-    $ret = array();
-    foreach ($data as $n => $v) {
-        $ret = array_merge($ret, csrf_flattenpost2(1, $n, $v));
-    }
-    return $ret;
-}
-function csrf_flattenpost2($level, $key, $data)
-{
-    if (!is_array($data)) {
-        return array($key => $data);
-    }
-    $ret = array();
-    foreach ($data as $n => $v) {
-        $nk = $level >= 1 ? $key."[$n]" : "[$n]";
-        $ret = array_merge($ret, csrf_flattenpost2($level+1, $nk, $v));
-    }
-    return $ret;
-}
-
 /**
  * @param $tokens is safe for HTML consumption
  */
 function csrf_callback($tokens)
 {
     // (yes, $tokens is safe to echo without escaping)
     header($_SERVER['SERVER_PROTOCOL'] . ' 403 Forbidden');
-    $data = '';
-    foreach (csrf_flattenpost($_POST) as $key => $value) {
-        if ($key == $GLOBALS['csrf']['input-name']) {
-            continue;
-        }
-        $data .= '<input type="hidden" name="'.htmlspecialchars($key).'" value="'.htmlspecialchars($value).'" />';
-    }
+
     echo "<html><head><title>CSRF check failed</title></head>
         <body>
         <p>CSRF check failed. Your form session may have expired, or you may not have
         cookies enabled.</p>
-        <form method='post' action=''>$data<input type='submit' value='Try again' /></form>
         <p>Debug: $tokens</p></body></html>
 ";
 }
@@ -398,16 +352,6 @@ function csrf_conf($key, $val)
     $GLOBALS['csrf'][$key] = $val;
 }
 
-/**
- * Starts a session if we're allowed to.
- */
-function csrf_start()
-{
-    if ($GLOBALS['csrf']['auto-session'] && session_status() == PHP_SESSION_NONE) {
-        session_start();
-    }
-}
-
 /**
  * Retrieves the secret, and generates one if necessary.
  */
@@ -469,6 +413,4 @@ function csrf_hash($value, $time = null)
     ob_start('csrf_ob_handler');
 }
 // Perform check
-if (!$GLOBALS['csrf']['defer']) {
-    csrf_check();
-}
+csrf_check();
diff --git a/src/www/guiconfig.inc b/src/www/guiconfig.inc
@@ -34,18 +34,17 @@ require_once("config.inc");
 
 /* THIS MUST BE ABOVE ALL OTHER CODE */
 if (empty($nocsrf)) {
-    function csrf_startup()
-    {
-      csrf_conf('rewrite-js', '/csrf/csrf-magic.js');
-      $timeout_minutes = isset($config['system']['webgui']['session_timeout']) ?  $config['system']['webgui']['session_timeout'] : 240;
-      csrf_conf('expires', $timeout_minutes * 60);
-    }
-    require_once('csrf/csrf-magic.php');
+    function csrf_startup() {
+        global $config;
 
-    // make sure the session is closed after executing csrf-magic
-    if (session_status() != PHP_SESSION_NONE) {
-        session_write_close();
+        csrf_conf('rewrite-js', '/csrf/csrf-magic.js');
+        $timeout_minutes = isset($config['system']['webgui']['session_timeout']) ? $config['system']['webgui']['session_timeout'] : 240;
+        csrf_conf('expires', $timeout_minutes * 60);
     }
+
+    session_start();
+    require_once('csrf/csrf-magic.php');
+    session_write_close();
 }
 
 function set_language()
