Skip to content
Permalink
Browse files Browse the repository at this point in the history
Enable CSRF in FORM by default
  • Loading branch information
jderusse committed Jan 28, 2022
1 parent 224eb7d commit f0ffb77
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 59 deletions.
Expand Up @@ -311,26 +311,6 @@ public function load(array $configs, ContainerBuilder $container)
$this->registerRequestConfiguration($config['request'], $container, $loader);
}

if ($this->isConfigEnabled($container, $config['form'])) {
if (!class_exists(Form::class)) {
throw new LogicException('Form support cannot be enabled as the Form component is not installed. Try running "composer require symfony/form".');
}

$this->formConfigEnabled = true;
$this->registerFormConfiguration($config, $container, $loader);

if (ContainerBuilder::willBeAvailable('symfony/validator', Validation::class, ['symfony/framework-bundle', 'symfony/form'])) {
$config['validation']['enabled'] = true;
} else {
$container->setParameter('validator.translation_domain', 'validators');

$container->removeDefinition('form.type_extension.form.validator');
$container->removeDefinition('form.type_guesser.validator');
}
} else {
$container->removeDefinition('console.command.form_debug');
}

if ($this->isConfigEnabled($container, $config['assets'])) {
if (!class_exists(\Symfony\Component\Asset\Package::class)) {
throw new LogicException('Asset support cannot be enabled as the Asset component is not installed. Try running "composer require symfony/asset".');
Expand All @@ -339,39 +319,6 @@ public function load(array $configs, ContainerBuilder $container)
$this->registerAssetsConfiguration($config['assets'], $container, $loader);
}

if ($this->messengerConfigEnabled = $this->isConfigEnabled($container, $config['messenger'])) {
$this->registerMessengerConfiguration($config['messenger'], $container, $loader, $config['validation']);
} else {
$container->removeDefinition('console.command.messenger_consume_messages');
$container->removeDefinition('console.command.messenger_debug');
$container->removeDefinition('console.command.messenger_stop_workers');
$container->removeDefinition('console.command.messenger_setup_transports');
$container->removeDefinition('console.command.messenger_failed_messages_retry');
$container->removeDefinition('console.command.messenger_failed_messages_show');
$container->removeDefinition('console.command.messenger_failed_messages_remove');
$container->removeDefinition('cache.messenger.restart_workers_signal');

if ($container->hasDefinition('messenger.transport.amqp.factory') && !class_exists(AmqpTransportFactory::class)) {
if (class_exists(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class)) {
$container->getDefinition('messenger.transport.amqp.factory')
->setClass(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class)
->addTag('messenger.transport_factory');
} else {
$container->removeDefinition('messenger.transport.amqp.factory');
}
}

if ($container->hasDefinition('messenger.transport.redis.factory') && !class_exists(RedisTransportFactory::class)) {
if (class_exists(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class)) {
$container->getDefinition('messenger.transport.redis.factory')
->setClass(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class)
->addTag('messenger.transport_factory');
} else {
$container->removeDefinition('messenger.transport.redis.factory');
}
}
}

if ($this->httpClientConfigEnabled = $this->isConfigEnabled($container, $config['http_client'])) {
$this->registerHttpClientConfiguration($config['http_client'], $container, $loader, $config['profiler']);
}
Expand All @@ -380,18 +327,12 @@ public function load(array $configs, ContainerBuilder $container)
$this->registerMailerConfiguration($config['mailer'], $container, $loader);
}

if ($this->notifierConfigEnabled = $this->isConfigEnabled($container, $config['notifier'])) {
$this->registerNotifierConfiguration($config['notifier'], $container, $loader);
}

$propertyInfoEnabled = $this->isConfigEnabled($container, $config['property_info']);
$this->registerValidationConfiguration($config['validation'], $container, $loader, $propertyInfoEnabled);
$this->registerHttpCacheConfiguration($config['http_cache'], $container, $config['http_method_override']);
$this->registerEsiConfiguration($config['esi'], $container, $loader);
$this->registerSsiConfiguration($config['ssi'], $container, $loader);
$this->registerFragmentsConfiguration($config['fragments'], $container, $loader);
$this->registerTranslatorConfiguration($config['translator'], $container, $loader, $config['default_locale']);
$this->registerProfilerConfiguration($config['profiler'], $container, $loader);
$this->registerWorkflowConfiguration($config['workflows'], $container, $loader);
$this->registerDebugConfiguration($config['php_errors'], $container, $loader);
$this->registerRouterConfiguration($config['router'], $container, $loader, $config['translator']['enabled_locales'] ?? []);
Expand Down Expand Up @@ -461,6 +402,72 @@ public function load(array $configs, ContainerBuilder $container)
}
$this->registerSecurityCsrfConfiguration($config['csrf_protection'], $container, $loader);

// form depends on csrf being registered
if ($this->isConfigEnabled($container, $config['form'])) {
if (!class_exists(Form::class)) {
throw new LogicException('Form support cannot be enabled as the Form component is not installed. Try running "composer require symfony/form".');
}

$this->formConfigEnabled = true;
$this->registerFormConfiguration($config, $container, $loader);

if (ContainerBuilder::willBeAvailable('symfony/validator', Validation::class, ['symfony/framework-bundle', 'symfony/form'])) {
$config['validation']['enabled'] = true;
} else {
$container->setParameter('validator.translation_domain', 'validators');

$container->removeDefinition('form.type_extension.form.validator');
$container->removeDefinition('form.type_guesser.validator');
}
} else {
$container->removeDefinition('console.command.form_debug');
}

// validation depends on form, annotations being registered
$this->registerValidationConfiguration($config['validation'], $container, $loader, $propertyInfoEnabled);

// messenger depends on validation being registered
if ($this->messengerConfigEnabled = $this->isConfigEnabled($container, $config['messenger'])) {
$this->registerMessengerConfiguration($config['messenger'], $container, $loader, $config['validation']);
} else {
$container->removeDefinition('console.command.messenger_consume_messages');
$container->removeDefinition('console.command.messenger_debug');
$container->removeDefinition('console.command.messenger_stop_workers');
$container->removeDefinition('console.command.messenger_setup_transports');
$container->removeDefinition('console.command.messenger_failed_messages_retry');
$container->removeDefinition('console.command.messenger_failed_messages_show');
$container->removeDefinition('console.command.messenger_failed_messages_remove');
$container->removeDefinition('cache.messenger.restart_workers_signal');

if ($container->hasDefinition('messenger.transport.amqp.factory') && !class_exists(AmqpTransportFactory::class)) {
if (class_exists(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class)) {
$container->getDefinition('messenger.transport.amqp.factory')
->setClass(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class)
->addTag('messenger.transport_factory');
} else {
$container->removeDefinition('messenger.transport.amqp.factory');
}
}

if ($container->hasDefinition('messenger.transport.redis.factory') && !class_exists(RedisTransportFactory::class)) {
if (class_exists(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class)) {
$container->getDefinition('messenger.transport.redis.factory')
->setClass(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class)
->addTag('messenger.transport_factory');
} else {
$container->removeDefinition('messenger.transport.redis.factory');
}
}
}

// notifier depends on messenger, mailer being registered
if ($this->notifierConfigEnabled = $this->isConfigEnabled($container, $config['notifier'])) {
$this->registerNotifierConfiguration($config['notifier'], $container, $loader);
}

// profiler depends on form, validation, translation, messenger, mailer, http-client, notifier being registered
$this->registerProfilerConfiguration($config['profiler'], $container, $loader);

$this->addAnnotatedClassesToCompile([
'**\\Controller\\',
'**\\Entity\\',
Expand Down
@@ -0,0 +1,11 @@
<?php

$container->loadFromExtension('framework', [
'form' => [
'legacy_error_messages' => false,
],
'session' => [
'storage_factory_id' => 'session.storage.factory.native',
'handler_id' => null,
],
]);
@@ -0,0 +1,13 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:framework="http://symfony.com/schema/dic/symfony"
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<framework:config>
<framework:form enabled="true" legacy-error-messages="false" />
<framework:session storage-factory-id="session.storage.factory.native" handler-id="null"/>
</framework:config>
</container>
@@ -0,0 +1,6 @@
framework:
form:
legacy_error_messages: false
session:
storage_factory_id: session.storage.factory.native
handler_id: null
Expand Up @@ -159,6 +159,18 @@ public function testCsrfProtectionForFormsEnablesCsrfProtectionAutomatically()
$this->assertTrue($container->hasDefinition('security.csrf.token_manager'));
}

public function testFormsCsrfIsEnabledByDefault()
{
if (class_exists(FullStack::class)) {
$this->markTestSkipped('testing with the FullStack prevents verifying default values');
}
$container = $this->createContainerFromFile('form_default_csrf');

$this->assertTrue($container->hasDefinition('security.csrf.token_manager'));
$this->assertTrue($container->hasParameter('form.type_extension.csrf.enabled'));
$this->assertTrue($container->getParameter('form.type_extension.csrf.enabled'));
}

public function testHttpMethodOverride()
{
$container = $this->createContainerFromFile('full');
Expand Down

6 comments on commit f0ffb77

@jk-
Copy link

@jk- jk- commented on f0ffb77 Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

@DanielRuf
Copy link

@DanielRuf DanielRuf commented on f0ffb77 May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symfony 4.3 is not affected, correct @fabpot?

GHSA-vvmr-8829-6whx

Because then the CPE-entry (in the "Known Affected Software Configurations" section) "up to 5.3.15" is misleading and confuses multiple scanners, which ultimately leads to false-positives.

https://nvd.nist.gov/vuln/detail/CVE-2022-23601#VulnChangeHistorySection

@stof
Copy link
Member

@stof stof commented on f0ffb77 May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanielRuf there are only 3 affected versions (see the GHSA link you gave, which contains the right info)

@DanielRuf
Copy link

@DanielRuf DanielRuf commented on f0ffb77 May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof correct. But then the CPE entries at https://nvd.nist.gov/vuln/detail/CVE-2022-23601 are wrong, which ultimately leads to false-positives caused by scanners.

Especially this one here:
image

@DanielRuf
Copy link

@DanielRuf DanielRuf commented on f0ffb77 May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the CNA is GitHub Inc., who could escalate this to them to fix the CPE entries?
Update: I have sent an email to nvd@nist.gov to let them know, that the CPE entries are not correct.

@DanielRuf
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still wrong CPE entries. GitHub support told me weeks / months ago, that they wrote NIST to fix the CPE entries. Seems there was no progress.

Please sign in to comment.