Commit 3fb2ec3f authored by alexpott's avatar alexpott

Issue #2242749 by znerol, torotil, rszrama, larowlan, dawehner, penyaskito,...

Issue #2242749 by znerol, torotil, rszrama, larowlan, dawehner, penyaskito, tim.plunkett, sun, Damien Tournoud, David_Rothstein,  effulgentsia: Fixed Port Form API security fix SA-CORE-2014-002 to Drupal 8.
parent 4e0e15b2
...@@ -182,7 +182,7 @@ services: ...@@ -182,7 +182,7 @@ services:
arguments: ['@request_stack', '@url_generator'] arguments: ['@request_stack', '@url_generator']
form_cache: form_cache:
class: Drupal\Core\Form\FormCache class: Drupal\Core\Form\FormCache
arguments: ['@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token'] arguments: ['@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token', '@logger.channel.form', '@config.factory', '@request_stack', '@page_cache_request_policy']
public: false # Private to form_builder public: false # Private to form_builder
keyvalue: keyvalue:
class: Drupal\Core\KeyValueStore\KeyValueFactory class: Drupal\Core\KeyValueStore\KeyValueFactory
......
<?php
/**
* @file
* Definition of Drupal\Core\Ajax\UpdateBuildIdCommand.
*/
namespace Drupal\Core\Ajax;
/**
* AJAX command for updating the value of a hidden form_build_id input element
* on a form. It requires the form passed in to have keys for both the old build
* ID in #build_id_old and the new build ID in #build_id.
*
* The primary use case for this Ajax command is to serve a new build ID to a
* form served from the cache to an anonymous user, preventing one anonymous
* user from accessing the form state of another anonymous user on Ajax enabled
* forms.
*
* This command is implemented by
* Drupal.AjaxCommands.prototype.update_build_id() defined in misc/ajax.js.
*O
* @ingroup ajax
*/
class UpdateBuildIdCommand implements CommandInterface {
/**
* Old build id.
*
* @var string
*/
protected $old;
/**
* New build id.
*
* @var string
*/
protected $new;
/**
* Constructs a UpdateBuildIdCommand object.
*
* @param string $old
* The old build_id.
* @param string $new
* The new build_id.
*/
public function __construct($old, $new) {
$this->old = $old;
$this->new = $new;
}
/**
* {@inheritdoc}
*/
public function render() {
return [
'command' => 'update_build_id',
'old' => $this->old,
'new' => $this->new,
];
}
}
...@@ -271,8 +271,8 @@ public function buildForm($form_id, FormStateInterface &$form_state) { ...@@ -271,8 +271,8 @@ public function buildForm($form_id, FormStateInterface &$form_state) {
exit; exit;
} }
// If this was a successful submission of a single-step form or the last step // If this was a successful submission of a single-step form or the last
// of a multi-step form, then self::processForm() issued a redirect to // step of a multi-step form, then self::processForm() issued a redirect to
// another page, or back to this page, but as a new request. Therefore, if // another page, or back to this page, but as a new request. Therefore, if
// we're here, it means that this is either a form being viewed initially // we're here, it means that this is either a form being viewed initially
// before any user input, or there was a validation error requiring the form // before any user input, or there was a validation error requiring the form
...@@ -291,17 +291,25 @@ public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form ...@@ -291,17 +291,25 @@ public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form
$form_state->setCached(); $form_state->setCached();
// If only parts of the form will be returned to the browser (e.g., Ajax or // If only parts of the form will be returned to the browser (e.g., Ajax or
// RIA clients), re-use the old #build_id to not require client-side code to // RIA clients), or if the form already had a new build ID regenerated when
// manually update the hidden 'build_id' input element. // it was retrieved from the form cache, reuse the existing #build_id.
// Otherwise, a new #build_id is generated, to not clobber the previous // Otherwise, a new #build_id is generated, to not clobber the previous
// build's data in the form cache; also allowing the user to go back to an // build's data in the form cache; also allowing the user to go back to an
// earlier build, make changes, and re-submit. // earlier build, make changes, and re-submit.
// @see self::prepareForm() // @see self::prepareForm()
$rebuild_info = $form_state->getRebuildInfo(); $rebuild_info = $form_state->getRebuildInfo();
if (isset($old_form['#build_id']) && !empty($rebuild_info['copy']['#build_id'])) { $enforce_old_build_id = isset($old_form['#build_id']) && !empty($rebuild_info['copy']['#build_id']);
$old_form_is_mutable_copy = isset($old_form['#build_id_old']);
if ($enforce_old_build_id || $old_form_is_mutable_copy) {
$form['#build_id'] = $old_form['#build_id']; $form['#build_id'] = $old_form['#build_id'];
if ($old_form_is_mutable_copy) {
$form['#build_id_old'] = $old_form['#build_id_old'];
}
} }
else { else {
if (isset($old_form['#build_id'])) {
$form['#build_id_old'] = $old_form['#build_id'];
}
$form['#build_id'] = 'form-' . Crypt::randomBytesBase64(); $form['#build_id'] = 'form-' . Crypt::randomBytesBase64();
} }
...@@ -486,17 +494,17 @@ public function processForm($form_id, &$form, FormStateInterface &$form_state) { ...@@ -486,17 +494,17 @@ public function processForm($form_id, &$form, FormStateInterface &$form_state) {
return; return;
} }
// If $form_state->isRebuilding() has been set and input has been processed // If $form_state->isRebuilding() has been set and input has been
// without validation errors, we are in a multi-step workflow that is not // processed without validation errors, we are in a multi-step workflow
// yet complete. A new $form needs to be constructed based on the changes // that is not yet complete. A new $form needs to be constructed based on
// made to $form_state during this request. Normally, a submit handler // the changes made to $form_state during this request. Normally, a submit
// sets $form_state->isRebuilding() if a fully executed form requires // handler sets $form_state->isRebuilding() if a fully executed form
// another step. However, for forms that have not been fully executed // requires another step. However, for forms that have not been fully
// (e.g., Ajax submissions triggered by non-buttons), there is no submit // executed (e.g., Ajax submissions triggered by non-buttons), there is no
// handler to set $form_state->isRebuilding(). It would not make sense to // submit handler to set $form_state->isRebuilding(). It would not make
// redisplay the identical form without an error for the user to correct, // sense to redisplay the identical form without an error for the user to
// so we also rebuild error-free non-executed forms, regardless of // correct, so we also rebuild error-free non-executed forms, regardless
// $form_state->isRebuilding(). // of $form_state->isRebuilding().
// @todo Simplify this logic; considering Ajax and non-HTML front-ends, // @todo Simplify this logic; considering Ajax and non-HTML front-ends,
// along with element-level #submit properties, it makes no sense to // along with element-level #submit properties, it makes no sense to
// have divergent form execution based on whether the triggering element // have divergent form execution based on whether the triggering element
......
...@@ -7,11 +7,16 @@ ...@@ -7,11 +7,16 @@
namespace Drupal\Core\Form; namespace Drupal\Core\Form;
use Drupal\Component\Utility\Crypt;
use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\SafeMarkup;
use Drupal\Core\Access\CsrfTokenGenerator; use Drupal\Core\Access\CsrfTokenGenerator;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface;
use Drupal\Core\PageCache\RequestPolicyInterface;
use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\AccountInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\RequestStack;
/** /**
* Encapsulates the caching of a form and its form state. * Encapsulates the caching of a form and its form state.
...@@ -48,6 +53,34 @@ class FormCache implements FormCacheInterface { ...@@ -48,6 +53,34 @@ class FormCache implements FormCacheInterface {
*/ */
protected $moduleHandler; protected $moduleHandler;
/**
* Logger channel.
*
* @var \Drupal\Core\Logger\LoggerChannelInterface
*/
protected $logger;
/**
* The config factory.
*
* @var \Drupal\Core\Config\ConfigFactoryInterface
*/
protected $configFactory;
/**
* The request stack.
*
* @var \Symfony\Component\HttpFoundation\RequestStack
*/
protected $requestStack;
/**
* A policy rule determining the cacheability of a request.
*
* @var \Drupal\Core\PageCache\RequestPolicyInterface
*/
protected $requestPolicy;
/** /**
* Constructs a new FormCache. * Constructs a new FormCache.
* *
...@@ -60,12 +93,24 @@ class FormCache implements FormCacheInterface { ...@@ -60,12 +93,24 @@ class FormCache implements FormCacheInterface {
* The current user. * The current user.
* @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token
* The CSRF token generator. * The CSRF token generator.
* @param \Psr\Log\LoggerInterface $logger
* A logger instance.
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* The configuration factory.
* @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
* The request stack.
* @param \Drupal\Core\PageCache\RequestPolicyInterface $request_policy
* A policy rule determining the cacheability of a request.
*/ */
public function __construct(KeyValueExpirableFactoryInterface $key_value_expirable_factory, ModuleHandlerInterface $module_handler, AccountInterface $current_user, CsrfTokenGenerator $csrf_token = NULL) { public function __construct(KeyValueExpirableFactoryInterface $key_value_expirable_factory, ModuleHandlerInterface $module_handler, AccountInterface $current_user, CsrfTokenGenerator $csrf_token, LoggerInterface $logger, ConfigFactoryInterface $config_factory, RequestStack $request_stack, RequestPolicyInterface $request_policy) {
$this->keyValueExpirableFactory = $key_value_expirable_factory; $this->keyValueExpirableFactory = $key_value_expirable_factory;
$this->moduleHandler = $module_handler; $this->moduleHandler = $module_handler;
$this->currentUser = $current_user; $this->currentUser = $current_user;
$this->logger = $logger;
$this->configFactory = $config_factory;
$this->csrfToken = $csrf_token; $this->csrfToken = $csrf_token;
$this->requestStack = $request_stack;
$this->requestPolicy = $request_policy;
} }
/** /**
...@@ -75,6 +120,18 @@ public function getCache($form_build_id, FormStateInterface $form_state) { ...@@ -75,6 +120,18 @@ public function getCache($form_build_id, FormStateInterface $form_state) {
if ($form = $this->keyValueExpirableFactory->get('form')->get($form_build_id)) { if ($form = $this->keyValueExpirableFactory->get('form')->get($form_build_id)) {
if ((isset($form['#cache_token']) && $this->csrfToken->validate($form['#cache_token'])) || (!isset($form['#cache_token']) && $this->currentUser->isAnonymous())) { if ((isset($form['#cache_token']) && $this->csrfToken->validate($form['#cache_token'])) || (!isset($form['#cache_token']) && $this->currentUser->isAnonymous())) {
$this->loadCachedFormState($form_build_id, $form_state); $this->loadCachedFormState($form_build_id, $form_state);
// Generate a new #build_id if the cached form was rendered on a
// cacheable page.
$build_info = $form_state->getBuildInfo();
if (!empty($build_info['immutable'])) {
$form['#build_id_old'] = $form['#build_id'];
$form['#build_id'] = 'form-' . Crypt::randomBytesBase64();
$form['form_build_id']['#value'] = $form['#build_id'];
$form['form_build_id']['#id'] = $form['#build_id'];
unset($build_info['immutable']);
$form_state->setBuildInfo($build_info);
}
return $form; return $form;
} }
} }
...@@ -106,8 +163,8 @@ protected function loadCachedFormState($form_build_id, FormStateInterface $form_ ...@@ -106,8 +163,8 @@ protected function loadCachedFormState($form_build_id, FormStateInterface $form_
require_once DRUPAL_ROOT . '/' . $file; require_once DRUPAL_ROOT . '/' . $file;
} }
} }
// Retrieve the list of previously known safe strings and store it // Retrieve the list of previously known safe strings and store it for
// for this request. // this request.
// @todo Ensure we are not storing an excessively large string list // @todo Ensure we are not storing an excessively large string list
// in: https://www.drupal.org/node/2295823 // in: https://www.drupal.org/node/2295823
$build_info += ['safe_strings' => []]; $build_info += ['safe_strings' => []];
...@@ -124,15 +181,29 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state) ...@@ -124,15 +181,29 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state)
// 6 hours cache life time for forms should be plenty. // 6 hours cache life time for forms should be plenty.
$expire = 21600; $expire = 21600;
// Ensure that the form build_id embedded in the form structure is the same
// as the one passed in as a parameter. This is an additional safety measure
// to prevent legacy code operating directly with form_get_cache and
// form_set_cache from accidentally overwriting immutable form state.
if (isset($form['#build_id']) && $form['#build_id'] != $form_build_id) {
$this->logger->error('Form build-id mismatch detected while attempting to store a form in the cache.');
return;
}
// Cache form structure. // Cache form structure.
if (isset($form)) { if (isset($form)) {
if ($this->currentUser->isAuthenticated()) { if ($this->currentUser->isAuthenticated()) {
$form['#cache_token'] = $this->csrfToken->get(); $form['#cache_token'] = $this->csrfToken->get();
} }
unset($form['#build_id_old']);
$this->keyValueExpirableFactory->get('form')->setWithExpire($form_build_id, $form, $expire); $this->keyValueExpirableFactory->get('form')->setWithExpire($form_build_id, $form, $expire);
} }
// Cache form state. // Cache form state.
if ($this->configFactory->get('system.performance')->get('cache.page.use_internal') && $this->isPageCacheable()) {
$form_state->addBuildInfo('immutable', TRUE);
}
// Store the known list of safe strings for form re-use. // Store the known list of safe strings for form re-use.
// @todo Ensure we are not storing an excessively large string list in: // @todo Ensure we are not storing an excessively large string list in:
// https://www.drupal.org/node/2295823 // https://www.drupal.org/node/2295823
...@@ -143,4 +214,14 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state) ...@@ -143,4 +214,14 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state)
} }
} }
/**
* Checks if the page is cacheable.
*
* @return bool
* TRUE is the page is cacheable, FALSE if not.
*/
protected function isPageCacheable() {
return ($this->requestPolicy->check($this->requestStack->getCurrentRequest()) === RequestPolicyInterface::ALLOW);
}
} }
...@@ -57,6 +57,12 @@ class FormState implements FormStateInterface { ...@@ -57,6 +57,12 @@ class FormState implements FormStateInterface {
* processed. * processed.
* - base_form_id: Identification for a base form, as declared in the form * - base_form_id: Identification for a base form, as declared in the form
* class's \Drupal\Core\Form\BaseFormIdInterface::getBaseFormId() method. * class's \Drupal\Core\Form\BaseFormIdInterface::getBaseFormId() method.
* - immutable: If this flag is set to TRUE, a new form build id is
* generated when the form is loaded from the cache. If it is subsequently
* saved to the cache again, it will have another cache id and therefore
* the original form and form-state will remain unaltered. This is
* important when page caching is enabled in order to prevent form state
* from leaking between anonymous users.
* *
* @var array * @var array
*/ */
......
...@@ -718,6 +718,13 @@ ...@@ -718,6 +718,13 @@
.filter(':odd').addClass('even'); .filter(':odd').addClass('even');
}, },
/**
* Command to update a form's build ID.
*/
update_build_id: function(ajax, response, status) {
$('input[name="form_build_id"][value="' + response.old + '"]').val(response.new);
},
/** /**
* Command to add css. * Command to add css.
* *
......
...@@ -43,7 +43,11 @@ public function upload(Request $request) { ...@@ -43,7 +43,11 @@ public function upload(Request $request) {
} }
try { try {
list($form, $form_state) = $this->getForm($request); /** @var $ajaxForm \Drupal\system\FileAjaxForm */
$ajaxForm = $this->getForm($request);
$form = $ajaxForm->getForm();
$form_state = $ajaxForm->getFormState();
$commands = $ajaxForm->getCommands();
} }
catch (HttpExceptionInterface $e) { catch (HttpExceptionInterface $e) {
// Invalid form_build_id. // Invalid form_build_id.
...@@ -80,6 +84,9 @@ public function upload(Request $request) { ...@@ -80,6 +84,9 @@ public function upload(Request $request) {
$settings = drupal_merge_js_settings($js['settings']['data']); $settings = drupal_merge_js_settings($js['settings']['data']);
$response = new AjaxResponse(); $response = new AjaxResponse();
foreach ($commands as $command) {
$response->addCommand($command, TRUE);
}
return $response->addCommand(new ReplaceCommand(NULL, $output, $settings)); return $response->addCommand(new ReplaceCommand(NULL, $output, $settings));
} }
......
...@@ -1897,6 +1897,12 @@ protected function drupalProcessAjaxResponse($content, array $ajax_response, arr ...@@ -1897,6 +1897,12 @@ protected function drupalProcessAjaxResponse($content, array $ajax_response, arr
break; break;
case 'add_css': case 'add_css':
break; break;
case 'update_build_id':
$buildId = $xpath->query('//input[@name="form_build_id" and @value="' . $command['old'] . '"]')->item(0);
if ($buildId) {
$buildId->setAttribute('value', $command['new']);
}
break;
} }
} }
$content = $dom->saveHTML(); $content = $dom->saveHTML();
......
...@@ -7,8 +7,10 @@ ...@@ -7,8 +7,10 @@
namespace Drupal\system\Controller; namespace Drupal\system\Controller;
use Drupal\Core\Ajax\UpdateBuildIdCommand;
use Drupal\Core\Form\FormState; use Drupal\Core\Form\FormState;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\system\FileAjaxForm;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
...@@ -63,7 +65,12 @@ public static function create(ContainerInterface $container) { ...@@ -63,7 +65,12 @@ public static function create(ContainerInterface $container) {
* @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface
*/ */
public function content(Request $request) { public function content(Request $request) {
list($form, $form_state) = $this->getForm($request); /** @var $ajaxForm \Drupal\system\FileAjaxForm */
$ajaxForm = $this->getForm($request);
$form = $ajaxForm->getForm();
$form_state = $ajaxForm->getFormState();
$commands = $ajaxForm->getCommands();
drupal_process_form($form['#form_id'], $form, $form_state); drupal_process_form($form['#form_id'], $form, $form_state);
// We need to return the part of the form (or some other content) that needs // We need to return the part of the form (or some other content) that needs
...@@ -81,7 +88,12 @@ public function content(Request $request) { ...@@ -81,7 +88,12 @@ public function content(Request $request) {
if (empty($callback) || !is_callable($callback)) { if (empty($callback) || !is_callable($callback)) {
throw new HttpException(500, t('Internal Server Error')); throw new HttpException(500, t('Internal Server Error'));
} }
return call_user_func_array($callback, array(&$form, &$form_state)); /** @var \Drupal\Core\Ajax\AjaxResponse $response */
$response = call_user_func_array($callback, [&$form, &$form_state]);
foreach ($commands as $command) {
$response->addCommand($command, TRUE);
}
return $response;
} }
/** /**
...@@ -93,14 +105,11 @@ public function content(Request $request) { ...@@ -93,14 +105,11 @@ public function content(Request $request) {
* @param \Symfony\Component\HttpFoundation\Request $request * @param \Symfony\Component\HttpFoundation\Request $request
* The current request object. * The current request object.
* *
* @return array * @return \Drupal\system\FileAjaxForm
* An array containing the $form and $form_state. Use the list() function * A wrapper object containing the $form, $form_state, $form_id,
* to break these apart: * $form_build_id and an initial list of Ajax $commands.
* @code
* list($form, $form_state, $form_id, $form_build_id) = $this->getForm();
* @endcode
* *
* @throws Symfony\Component\HttpKernel\Exception\HttpExceptionInterface * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface
*/ */
protected function getForm(Request $request) { protected function getForm(Request $request) {
$form_state = new FormState(); $form_state = new FormState();
...@@ -118,6 +127,17 @@ protected function getForm(Request $request) { ...@@ -118,6 +127,17 @@ protected function getForm(Request $request) {
throw new BadRequestHttpException(); throw new BadRequestHttpException();
} }
// When a page level cache is enabled, the form-build id might have been
// replaced from within form_get_cache. If this is the case, it is also
// necessary to update it in the browser by issuing an appropriate Ajax
// command.
$commands = [];
if (isset($form['#build_id_old']) && $form['#build_id_old'] != $form['#build_id']) {
// If the form build ID has changed, issue an Ajax command to update it.
$commands[] = new UpdateBuildIdCommand($form['#build_id_old'], $form['#build_id']);
$form_build_id = $form['#build_id'];
}
// Since some of the submit handlers are run, redirects need to be disabled. // Since some of the submit handlers are run, redirects need to be disabled.
$form_state->disableRedirect(); $form_state->disableRedirect();
...@@ -129,12 +149,12 @@ protected function getForm(Request $request) { ...@@ -129,12 +149,12 @@ protected function getForm(Request $request) {
'#action' => TRUE, '#action' => TRUE,
]); ]);
// The form needs to be processed; prepare for that by setting a few internal // The form needs to be processed; prepare for that by setting a few
// variables. // internal variables.
$form_state->setUserInput($request->request->all()); $form_state->setUserInput($request->request->all());
$form_id = $form['#form_id']; $form_id = $form['#form_id'];
return array($form, $form_state, $form_id, $form_build_id); return new FileAjaxForm($form, $form_state, $form_id, $form_build_id, $commands);
} }
} }
<?php
/**
* @file
* Contains Drupal\system\FileAjaxForm.
*/
namespace Drupal\system;
use Drupal\Core\Form\FormStateInterface;
/**
* Wrapper for Ajax forms data and commands, avoiding a multi-return-value tuple.
*
* @ingroup ajax
*/
class FileAjaxForm {
/**
* The form to cache.
*
* @var array
*/
protected $form;
/**
* The form state.
*
* @var \Drupal\Core\Form\FormStateInterface
*/
protected $formState;
/**
* The unique form ID.
*
* @var string
*/
protected $formId;
/**
* The unique form build ID.
*
* @var string
*/
protected $formBuildId;
/**
* The array of ajax commands.
*
* @var \Drupal\Core\Ajax\CommandInterface[]
*/
protected $commands;
/**
* Constructs a FileAjaxForm object.
*
* @param array $form
* The form definition.
* @param \Drupal\Core\Form\FormStateInterface $form_state