Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

FIX Prevent DOS by checking for env and admin on ?flush=1 (#1692) in 2.4 #2246

Merged
merged 2 commits into from

2 participants

@hafriedlander

Ensure that flush=1 only causes an actual flush if one of the following is true

  • You are in dev mode
  • You are logged in as an admin
  • An error occurred while attempted to start up the page

Fixes #1692. This is the version for 2.4. The version for 3.0 is at #2243

@hafriedlander

This should be ready to go now, but needs checking in PHP 5.2

@sminnee sminnee merged commit b774db4 into silverstripe:2.4

1 check failed

Details default Scrutinizer: 13 Comments, 0 Changed Files — Travis: Failed
@sminnee
Owner

Doh, I merged this without noticing an issue. require_once(core/TempPath.php): doesn't exist in 2.4 and it's trying to include it.

@hafriedlander

Flush - there's two requests, and the first one creates TempPath.php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
60 core/Core.php
@@ -173,6 +173,7 @@ function array_fill_keys($keys,$value) {
/**
* Define the temporary folder if it wasn't defined yet
*/
+require_once('core/TempPath.php');
if(!defined('TEMP_FOLDER')) {
define('TEMP_FOLDER', getTempFolder());
}
@@ -254,65 +255,6 @@ function sapphire_autoload($className) {
///////////////////////////////////////////////////////////////////////////////
// HELPER FUNCTIONS
-function getSysTempDir() {
- if(function_exists('sys_get_temp_dir')) {
- $sysTmp = sys_get_temp_dir();
- } elseif(isset($_ENV['TMP'])) {
- $sysTmp = $_ENV['TMP'];
- } else {
- $tmpFile = tempnam('adfadsfdas','');
- unlink($tmpFile);
- $sysTmp = dirname($tmpFile);
- }
- return $sysTmp;
-}
-
-/**
- * Returns the temporary folder that sapphire/silverstripe should use for its cache files
- * This is loaded into the TEMP_FOLDER define on start up
- *
- * @param $base The base path to use as the basis for the temp folder name. Defaults to BASE_PATH,
- * which is usually fine; however, the $base argument can be used to help test.
- */
-function getTempFolder($base = null) {
- if(!$base) $base = BASE_PATH;
-
- if($base) {
- $cachefolder = "silverstripe-cache" . str_replace(array(' ', "/", ":", "\\"), "-", $base);
- } else {
- $cachefolder = "silverstripe-cache";
- }
-
- $ssTmp = BASE_PATH . "/silverstripe-cache";
- if(@file_exists($ssTmp)) {
- return $ssTmp;
- }
-
- $sysTmp = getSysTempDir();
- $worked = true;
- $ssTmp = "$sysTmp/$cachefolder";
-
- if(!@file_exists($ssTmp)) {
- @$worked = mkdir($ssTmp);
- }
-
- if(!$worked) {
- $ssTmp = BASE_PATH . "/silverstripe-cache";
- $worked = true;
- if(!@file_exists($ssTmp)) {
- @$worked = mkdir($ssTmp);
- }
- }
-
- if(!$worked) {
- user_error("Permission problem gaining access to a temp folder. " .
- "Please create a folder named silverstripe-cache in the base folder " .
- "of the installation and ensure it has the correct permissions", E_USER_ERROR);
- }
-
- return $ssTmp;
-}
-
/**
* Return the file where that class is stored.
*
View
60 core/TempPath.php
@@ -0,0 +1,60 @@
+<?php
+
+function getSysTempDir() {
+ if(function_exists('sys_get_temp_dir')) {
+ $sysTmp = sys_get_temp_dir();
+ } elseif(isset($_ENV['TMP'])) {
+ $sysTmp = $_ENV['TMP'];
+ } else {
+ $tmpFile = tempnam('adfadsfdas','');
+ unlink($tmpFile);
+ $sysTmp = dirname($tmpFile);
+ }
+ return $sysTmp;
+}
+
+/**
+ * Returns the temporary folder that sapphire/silverstripe should use for its cache files
+ * This is loaded into the TEMP_FOLDER define on start up
+ *
+ * @param $base The base path to use as the basis for the temp folder name. Defaults to BASE_PATH,
+ * which is usually fine; however, the $base argument can be used to help test.
+ */
+function getTempFolder($base = null) {
+ if(!$base) $base = BASE_PATH;
+
+ if($base) {
+ $cachefolder = "silverstripe-cache" . str_replace(array(' ', "/", ":", "\\"), "-", $base);
+ } else {
+ $cachefolder = "silverstripe-cache";
+ }
+
+ $ssTmp = BASE_PATH . "/silverstripe-cache";
+ if(@file_exists($ssTmp)) {
+ return $ssTmp;
+ }
+
+ $sysTmp = getSysTempDir();
+ $worked = true;
+ $ssTmp = "$sysTmp/$cachefolder";
+
+ if(!@file_exists($ssTmp)) {
+ @$worked = mkdir($ssTmp);
+ }
+
+ if(!$worked) {
+ $ssTmp = BASE_PATH . "/silverstripe-cache";
+ $worked = true;
+ if(!@file_exists($ssTmp)) {
+ @$worked = mkdir($ssTmp);
+ }
+ }
+
+ if(!$worked) {
+ user_error("Permission problem gaining access to a temp folder. " .
+ "Please create a folder named silverstripe-cache in the base folder " .
+ "of the installation and ensure it has the correct permissions", E_USER_ERROR);
+ }
+
+ return $ssTmp;
+}
View
121 core/startup/ErrorControlChain.php
@@ -0,0 +1,121 @@
+<?php
+
+/**
+ * Class ErrorControlChain
+ *
+ * Runs a set of steps, optionally suppressing (but recording) any errors (even fatal ones) that occur in each step.
+ * If an error does occur, subsequent steps are normally skipped, but can optionally be run anyway
+ *
+ * Normal errors are suppressed even past the end of the chain. Fatal errors are only suppressed until the end
+ * of the chain - the request will then die silently.
+ *
+ * The exception is if an error occurs and BASE_URL is not yet set - in that case the error is never suppressed.
+ *
+ * Usage:
+ *
+ * $chain = new ErrorControlChain();
+ * $chain->then($callback1)->then($callback2)->then(true, $callback3)->execute();
+ *
+ * WARNING: This class is experimental and designed specifically for use pre-startup in main.php
+ * It will likely be heavily refactored before the release of 3.2
+ */
+class ErrorControlChain {
+ protected $error = false;
+ protected $steps = array();
+
+ protected $suppression = true;
+
+ /** We can't unregister_shutdown_function, so this acts as a flag to enable handling */
+ protected $handleFatalErrors = false;
+
+ public function hasErrored() {
+ return $this->error;
+ }
+
+ public function setErrored($error) {
+ $this->error = (bool)$error;
+ }
+
+ public function setSuppression($suppression) {
+ $this->suppression = (bool)$suppression;
+ }
+
+ /**
+ * Add this callback to the chain of callbacks to call along with the state
+ * that $error must be in this point in the chain for the callback to be called
+ *
+ * @param $callback - The callback to call
+ * @param $onErrorState - false if only call if no errors yet, true if only call if already errors, null for either
+ * @return $this
+ */
+ public function then($callback, $onErrorState = false) {
+ $this->steps[] = array(
+ 'callback' => $callback,
+ 'onErrorState' => $onErrorState
+ );
+ return $this;
+ }
+
+ public function thenWhileGood($callback) {
+ return $this->then($callback, false);
+ }
+
+ public function thenIfErrored($callback) {
+ return $this->then($callback, true);
+ }
+
+ public function thenAlways($callback) {
+ return $this->then($callback, null);
+ }
+
+ public function handleError() {
+ if ($this->suppression && defined('BASE_URL')) throw new Exception('Generic Error');
+ else return false;
+ }
+
+ protected function lastErrorWasFatal() {
+ $error = error_get_last();
+ return $error && $error['type'] == 1;
+ }
+
+ public function handleFatalError() {
+ if ($this->handleFatalErrors && $this->suppression && defined('BASE_URL')) {
+ if ($this->lastErrorWasFatal()) {
+ ob_clean();
+ $this->error = true;
+ $this->step();
+ }
+ }
+ }
+
+ public function execute() {
+ set_error_handler(array($this, 'handleError'), error_reporting());
+ register_shutdown_function(array($this, 'handleFatalError'));
+ $this->handleFatalErrors = true;
+
+ $this->step();
+ }
+
+ protected function step() {
+ if ($this->steps) {
+ $step = array_shift($this->steps);
+
+ if ($step['onErrorState'] === null || $step['onErrorState'] === $this->error) {
+ try {
+ call_user_func($step['callback'], $this);
+ }
+ catch (Exception $e) {
+ if ($this->suppression && defined('BASE_URL')) $this->error = true;
+ else throw $e;
+ }
+ }
+
+ $this->step();
+ }
+ else {
+ // Now clean up
+ $this->handleFatalErrors = false;
+ restore_error_handler();
+ }
+ }
+}
View
113 core/startup/ParameterConfirmationToken.php
@@ -0,0 +1,113 @@
+<?php
+
+/**
+ * Class ParameterConfirmationToken
+ *
+ * When you need to use a dangerous GET parameter that needs to be set before core/Core.php is
+ * established, this class takes care of allowing some other code of confirming the parameter,
+ * by generating a one-time-use token & redirecting with that token included in the redirected URL
+ *
+ * WARNING: This class is experimental and designed specifically for use pre-startup in main.php
+ * It will likely be heavily refactored before the release of 3.2
+ */
+class ParameterConfirmationToken {
+ protected $parameterName = null;
+ protected $parameter = null;
+ protected $token = null;
+
+ protected function pathForToken($token) {
+ if (defined('BASE_PATH')) {
+ $basepath = BASE_PATH;
+ }
+ else {
+ $basepath = rtrim(dirname(dirname(dirname(dirname(__FILE__)))), DIRECTORY_SEPARATOR);
+ }
+
+ require_once('core/TempPath.php');
+ $tempfolder = getTempFolder($basepath ? $basepath : DIRECTORY_SEPARATOR);
+
+ return $tempfolder.'/token_'.preg_replace('/[^a-z0-9]+/', '', $token);
+ }
+
+ protected function genToken() {
+ // Generate a new random token (as random as possible)
+ require_once('security/RandomGenerator.php');
+ $rg = new RandomGenerator();
+ $token = $rg->randomToken('md5');
+
+ // Store a file in the session save path (safer than /tmp, as open_basedir might limit that)
+ file_put_contents($this->pathForToken($token), $token);
+
+ return $token;
+ }
+
+ protected function checkToken($token) {
+ $file = $this->pathForToken($token);
+ $content = null;
+
+ if (file_exists($file)) {
+ $content = file_get_contents($file);
+ unlink($file);
+ }
+
+ return $content == $token;
+ }
+
+ public function __construct($parameterName) {
+ // Store the parameter name
+ $this->parameterName = $parameterName;
+ // Store the parameter value
+ $this->parameter = isset($_GET[$parameterName]) ? $_GET[$parameterName] : null;
+ // Store the token
+ $this->token = isset($_GET[$parameterName.'token']) ? $_GET[$parameterName.'token'] : null;
+
+ // If a token was provided, but isn't valid, just throw a 403
+ if ($this->token && (!$this->checkToken($this->token))) {
+ header("HTTP/1.0 403 Forbidden", true, 403);
+ die;
+ }
+ }
+
+ public function parameterProvided() {
+ return $this->parameter !== null;
+ }
+
+ public function tokenProvided() {
+ return $this->token !== null;
+ }
+
+ public function params() {
+ return array(
+ $this->parameterName => $this->parameter,
+ $this->parameterName.'token' => $this->genToken()
+ );
+ }
+
+ public function reloadWithToken() {
+ global $url;
+
+ // Are we http or https?
+ $proto = 'http';
+
+ if(isset($_SERVER['HTTP_X_FORWARDED_PROTOCOL'])) {
+ if(strtolower($_SERVER['HTTP_X_FORWARDED_PROTOCOL']) == 'https') $proto = 'https';
+ }
+
+ if((!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) $proto = 'https';
+ if(isset($_SERVER['SSL'])) $proto = 'https';
+
+ // What's our host
+ $host = $_SERVER['HTTP_HOST'];
+
+ // What's our GET params (ensuring they include the original parameter + a new token)
+ $params = array_merge($_GET, $this->params());
+ unset($params['url']);
+
+ // Join them all together into the original URL
+ $location = "$proto://" . $host . BASE_URL . $url . ($params ? '?'.http_build_query($params) : '');
+
+ // And redirect
+ header('location: '.$location, true, 302);
+ die;
+ }
+}
View
23 docs/en/changelogs/2.4.11.md
@@ -0,0 +1,23 @@
+# 2.4.11 (Not yet released)
+
+## Overview
+
+ * Security: Require ADMIN for `?flush=1` (stop denial of service attacks)
+ ([#1692](https://github.com/silverstripe/silverstripe-framework/issues/1692))
+
+## Details
+
+### Security: Require ADMIN for ?flush=1 and ?flush=all
+
+Flushing the various manifests (class, template, config) is performed through a GET
+parameter (`flush=1`). Since this action requires more server resources than normal requests,
+it can facilitate [denial-of-service attacks](https://en.wikipedia.org/wiki/Denial-of-service_attack).
+
+To prevent this, main.php now checks and only allows the flush parameter in the following cases:
+
+ * The [environment](/topics/environment-management) is in "dev mode"
+ * A user is logged in with ADMIN permissions
+ * An error occurs during startup
+
+This applies to both `flush=1` and `flush=all`but only through web requests made through main.php - CLI requests,
+or any other request that goes through a custom start up script will still process all flush requests as normal.
View
154 main.php
@@ -54,55 +54,129 @@
* @see Director::direct()
*/
+require_once('core/startup/ErrorControlChain.php');
+require_once('core/startup/ParameterConfirmationToken.php');
-/**
- * Include Sapphire's core code
- */
-require_once("core/Core.php");
+class SilverStripeMain {
+ static $token;
-if (function_exists('mb_http_output')) {
- mb_http_output('UTF-8');
- mb_internal_encoding('UTF-8');
-}
+ static function filterFlush($chain) {
+ self::$token = new ParameterConfirmationToken('flush');
-Session::start();
+ if (isset($_GET['flush']) && !self::$token->tokenProvided()) {
+ unset($_GET['flush']);
+ }
+ else {
+ $chain->setSuppression(false);
+ }
+ }
-// IIS will sometimes generate this.
-if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) {
- $_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_ORIGINAL_URL'];
-}
+ static function includeCore() {
+ /**
+ * Include Sapphire's core code
+ */
+ require_once("core/Core.php");
-// Apache rewrite rules use this
-if (isset($_GET['url'])) {
- $url = $_GET['url'];
- // IIS includes get variables in url
- $i = strpos($url, '?');
- if($i !== false) {
- $url = substr($url, 0, $i);
+ if (function_exists('mb_http_output')) {
+ mb_http_output('UTF-8');
+ mb_internal_encoding('UTF-8');
+ }
+
+ Session::start();
}
-
-// Lighttpd uses this
-} else {
- if(strpos($_SERVER['REQUEST_URI'],'?') !== false) {
- list($url, $query) = explode('?', $_SERVER['REQUEST_URI'], 2);
- parse_str($query, $_GET);
- if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET);
- } else {
- $url = $_SERVER["REQUEST_URI"];
+
+ static function parseURL() {
+ global $url;
+
+ // IIS will sometimes generate this.
+ if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) {
+ $_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_ORIGINAL_URL'];
+ }
+
+ // Apache rewrite rules use this
+ if (isset($_GET['url'])) {
+ $url = $_GET['url'];
+ // IIS includes get variables in url
+ $i = strpos($url, '?');
+ if($i !== false) {
+ $url = substr($url, 0, $i);
+ }
+
+ // Lighttpd uses this
+ } else {
+ if(strpos($_SERVER['REQUEST_URI'],'?') !== false) {
+ list($url, $query) = explode('?', $_SERVER['REQUEST_URI'], 2);
+ parse_str($query, $_GET);
+ if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET);
+ } else {
+ $url = $_SERVER["REQUEST_URI"];
+ }
+ }
+
+ // Remove base folders from the URL if webroot is hosted in a subfolder
+ if (substr(strtolower($url), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) $url = substr($url, strlen(BASE_URL));
}
-}
-// Remove base folders from the URL if webroot is hosted in a subfolder
-if (substr(strtolower($url), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) $url = substr($url, strlen(BASE_URL));
+ static function startupDatabase() {
+ if (isset($_GET['debug_profile'])) {
+ Profiler::init();
+ Profiler::mark('all_execution');
+ Profiler::mark('main.php init');
+ }
-if (isset($_GET['debug_profile'])) {
- Profiler::init();
- Profiler::mark('all_execution');
- Profiler::mark('main.php init');
+ // Connect to database
+ require_once("core/model/DB.php");
+ global $databaseConfig;
+
+ if (isset($_GET['debug_profile'])) Profiler::mark('DB::connect');
+ if ($databaseConfig) DB::connect($databaseConfig);
+ if (isset($_GET['debug_profile'])) Profiler::unmark('DB::connect');
+ }
+
+ static function flushIfAllowed() {
+ if (self::$token->parameterProvided() && !self::$token->tokenProvided()) {
+ // First, check if we're in dev mode, or the database doesn't have any security data
+ $canFlush = Director::isDev() || !Security::database_is_ready();
+
+ // Otherwise, we start up the session if needed, then check for admin
+ if (!$canFlush) {
+ if(!isset($_SESSION) && (isset($_COOKIE[session_name()]) || isset($_REQUEST[session_name()]))) {
+ Session::start();
+ }
+
+ if (Permission::check('ADMIN')) {
+ $canFlush = true;
+ }
+ else {
+ $loginPage = Director::absoluteURL(Config::inst()->get('Security', 'login_url'));
+ $loginPage .= "?BackURL=" . urlencode($_SERVER['REQUEST_URI']);
+
+ header('location: '.$loginPage, true, 302);
+ die;
+ }
+ }
+
+ // And if we can flush, reload with an authority token
+ if ($canFlush) self::$token->reloadWithToken();
+ }
+ }
+
+ static function flushIfErrored() {
+ if (self::$token->parameterProvided() && !self::$token->tokenProvided()) {
+ self::$token->reloadWithToken();
+ }
+ }
}
-// Connect to database
-require_once("core/model/DB.php");
+$chain = new ErrorControlChain();
+$chain
+ ->then(array('SilverStripeMain', 'filterFlush'))
+ ->then(array('SilverStripeMain', 'includeCore'))
+ ->thenAlways(array('SilverStripeMain', 'parseURL'))
+ ->then(array('SilverStripeMain', 'startupDatabase'))
+ ->then(array('SilverStripeMain', 'flushIfAllowed'))
+ ->thenIfErrored(array('SilverStripeMain', 'flushIfErrored'))
+ ->execute();
// Redirect to the installer if no database is selected
if(!isset($databaseConfig) || !isset($databaseConfig['database']) || !$databaseConfig['database']) {
@@ -117,10 +191,6 @@
die();
}
-if (isset($_GET['debug_profile'])) Profiler::mark('DB::connect');
-DB::connect($databaseConfig);
-if (isset($_GET['debug_profile'])) Profiler::unmark('DB::connect');
-
if (isset($_GET['debug_profile'])) Profiler::unmark('main.php init');
// Direct away - this is the "main" function, that hands control to the appropriate controller
Something went wrong with that request. Please try again.