Skip to content

Commit ea2447f

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '2.8' into 3.3
* 2.8: fixed CS fixed CS [Security] Namespace generated CSRF tokens depending of the current scheme ensure that submitted data are uploaded files [Console] remove dead code bumped Symfony version to 2.8.31 updated VERSION for 2.8.30 updated CHANGELOG for 2.8.30 bumped Symfony version to 2.7.38 updated VERSION for 2.7.37 updated CHANGELOG for 2.7.37 [Security] Validate redirect targets using the session cookie domain prevent bundle readers from breaking out of paths
2 parents 73ef74a + 44c5d7f commit ea2447f

File tree

24 files changed

+598
-138
lines changed

24 files changed

+598
-138
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
<service id="security.csrf.token_manager" class="Symfony\Component\Security\Csrf\CsrfTokenManager" public="true">
1919
<argument type="service" id="security.csrf.token_generator" />
2020
<argument type="service" id="security.csrf.token_storage" />
21+
<argument type="service" id="request_stack" on-invalid="ignore" />
2122
</service>
2223
<service id="Symfony\Component\Security\Csrf\CsrfTokenManagerInterface" alias="security.csrf.token_manager" />
2324
</services>

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\ContainerBuilder;
15+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
16+
17+
/**
18+
* Uses the session domain to restrict allowed redirection targets.
19+
*
20+
* @author Nicolas Grekas <[email protected]>
21+
*/
22+
class AddSessionDomainConstraintPass implements CompilerPassInterface
23+
{
24+
/**
25+
* {@inheritdoc}
26+
*/
27+
public function process(ContainerBuilder $container)
28+
{
29+
if (!$container->hasParameter('session.storage.options') || !$container->has('security.http_utils')) {
30+
return;
31+
}
32+
33+
$sessionOptions = $container->getParameter('session.storage.options');
34+
$domainRegexp = empty($sessionOptions['cookie_domain']) ? '%s' : sprintf('(?:%%s|(?:.+\.)?%s)', preg_quote(trim($sessionOptions['cookie_domain'], '.')));
35+
$domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp;
36+
37+
$container->findDefinition('security.http_utils')->addArgument(sprintf('{^%s$}i', $domainRegexp));
38+
}
39+
}

‎src/Symfony/Bundle/SecurityBundle/SecurityBundle.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313

1414
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\JsonLoginFactory;
1515
use Symfony\Component\HttpKernel\Bundle\Bundle;
16+
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
1617
use Symfony\Component\DependencyInjection\ContainerBuilder;
1718
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
19+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
1820
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
1921
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginLdapFactory;
2022
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory;
@@ -57,5 +59,6 @@ public function build(ContainerBuilder $container)
5759
$extension->addUserProviderFactory(new InMemoryFactory());
5860
$extension->addUserProviderFactory(new LdapFactory());
5961
$container->addCompilerPass(new AddSecurityVotersPass());
62+
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
6063
}
6164
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Compiler;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension;
16+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
17+
use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
18+
use Symfony\Component\DependencyInjection\ContainerBuilder;
19+
use Symfony\Component\HttpFoundation\Request;
20+
21+
class AddSessionDomainConstraintPassTest extends TestCase
22+
{
23+
public function testSessionCookie()
24+
{
25+
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.', 'cookie_secure' => true));
26+
27+
$utils = $container->get('security.http_utils');
28+
$request = Request::create('/', 'get');
29+
30+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
31+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
32+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
33+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
34+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
35+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
36+
}
37+
38+
public function testSessionNoDomain()
39+
{
40+
$container = $this->createContainer(array('cookie_secure' => true));
41+
42+
$utils = $container->get('security.http_utils');
43+
$request = Request::create('/', 'get');
44+
45+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
46+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
47+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
48+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
49+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
50+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
51+
}
52+
53+
public function testSessionNoSecure()
54+
{
55+
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.'));
56+
57+
$utils = $container->get('security.http_utils');
58+
$request = Request::create('/', 'get');
59+
60+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
61+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
62+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
63+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
64+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
65+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
66+
}
67+
68+
public function testSessionNoSecureAndNoDomain()
69+
{
70+
$container = $this->createContainer(array());
71+
72+
$utils = $container->get('security.http_utils');
73+
$request = Request::create('/', 'get');
74+
75+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
76+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
77+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
78+
$this->assertTrue($utils->createRedirectResponse($request, 'http://localhost/foo')->isRedirect('http://localhost/foo'));
79+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
80+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
81+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
82+
}
83+
84+
public function testNoSession()
85+
{
86+
$container = $this->createContainer(null);
87+
88+
$utils = $container->get('security.http_utils');
89+
$request = Request::create('/', 'get');
90+
91+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
92+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
93+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
94+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('https://www.localhost/foo'));
95+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
96+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://pirate.com/foo'));
97+
}
98+
99+
private function createContainer($sessionStorageOptions)
100+
{
101+
$container = new ContainerBuilder();
102+
$container->setParameter('kernel.cache_dir', __DIR__);
103+
$container->setParameter('kernel.charset', 'UTF-8');
104+
$container->setParameter('kernel.container_class', 'cc');
105+
$container->setParameter('kernel.debug', true);
106+
$container->setParameter('kernel.root_dir', __DIR__);
107+
$container->setParameter('kernel.secret', __DIR__);
108+
if (null !== $sessionStorageOptions) {
109+
$container->setParameter('session.storage.options', $sessionStorageOptions);
110+
}
111+
$container->setParameter('request_listener.http_port', 80);
112+
$container->setParameter('request_listener.https_port', 443);
113+
114+
$config = array(
115+
'security' => array(
116+
'providers' => array('some_provider' => array('id' => 'foo')),
117+
'firewalls' => array('some_firewall' => array('security' => false)),
118+
),
119+
);
120+
121+
$ext = new FrameworkExtension();
122+
$ext->load(array(), $container);
123+
124+
$ext = new SecurityExtension();
125+
$ext->load($config, $container);
126+
127+
(new AddSessionDomainConstraintPass())->process($container);
128+
129+
return $container;
130+
}
131+
}

‎src/Symfony/Component/Console/Application.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,13 @@ public function run(InputInterface $input = null, OutputInterface $output = null
120120
try {
121121
$e = null;
122122
$exitCode = $this->doRun($input, $output);
123-
} catch (\Exception $x) {
124-
$e = $x;
125-
} catch (\Throwable $x) {
126-
$e = new FatalThrowableError($x);
123+
} catch (\Exception $e) {
124+
} catch (\Throwable $e) {
127125
}
128126

129127
if (null !== $e) {
130-
if (!$this->catchExceptions || !$x instanceof \Exception) {
131-
throw $x;
128+
if (!$this->catchExceptions || !$e instanceof \Exception) {
129+
throw $e;
132130
}
133131

134132
if ($output instanceof ConsoleOutputInterface) {

‎src/Symfony/Component/Form/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ CHANGELOG
4848
* moved data trimming logic of TrimListener into StringUtil
4949
* [BC BREAK] When registering a type extension through the DI extension, the tag alias has to match the actual extended type.
5050

51+
2.7.38
52+
------
53+
54+
* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
55+
5156
2.7.0
5257
-----
5358

‎src/Symfony/Component/Form/Extension/Core/Type/FileType.php

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,35 @@ class FileType extends AbstractType
2727
*/
2828
public function buildForm(FormBuilderInterface $builder, array $options)
2929
{
30-
if ($options['multiple']) {
31-
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
32-
$form = $event->getForm();
33-
$data = $event->getData();
30+
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
31+
$form = $event->getForm();
32+
$requestHandler = $form->getConfig()->getRequestHandler();
33+
$data = null;
34+
35+
if ($options['multiple']) {
36+
$data = array();
37+
38+
foreach ($event->getData() as $file) {
39+
if ($requestHandler->isFileUpload($file)) {
40+
$data[] = $file;
41+
}
42+
}
3443

3544
// submitted data for an input file (not required) without choosing any file
36-
if (array(null) === $data) {
45+
if (array(null) === $data || array() === $data) {
3746
$emptyData = $form->getConfig()->getEmptyData();
3847

3948
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
40-
$event->setData($data);
4149
}
42-
});
43-
}
50+
51+
$event->setData($data);
52+
} elseif (!$requestHandler->isFileUpload($event->getData())) {
53+
$emptyData = $form->getConfig()->getEmptyData();
54+
55+
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
56+
$event->setData($data);
57+
}
58+
});
4459
}
4560

4661
/**

‎src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Form\FormInterface;
1717
use Symfony\Component\Form\RequestHandlerInterface;
1818
use Symfony\Component\Form\Util\ServerParams;
19+
use Symfony\Component\HttpFoundation\File\File;
1920
use Symfony\Component\HttpFoundation\Request;
2021

2122
/**
@@ -28,9 +29,6 @@ class HttpFoundationRequestHandler implements RequestHandlerInterface
2829
{
2930
private $serverParams;
3031

31-
/**
32-
* {@inheritdoc}
33-
*/
3432
public function __construct(ServerParams $serverParams = null)
3533
{
3634
$this->serverParams = $serverParams ?: new ServerParams();
@@ -109,4 +107,9 @@ public function handleRequest(FormInterface $form, $request = null)
109107

110108
$form->submit($data, 'PATCH' !== $method);
111109
}
110+
111+
public function isFileUpload($data)
112+
{
113+
return $data instanceof File;
114+
}
112115
}

‎src/Symfony/Component/Form/NativeRequestHandler.php

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,6 @@ class NativeRequestHandler implements RequestHandlerInterface
2323
{
2424
private $serverParams;
2525

26-
/**
27-
* {@inheritdoc}
28-
*/
29-
public function __construct(ServerParams $params = null)
30-
{
31-
$this->serverParams = $params ?: new ServerParams();
32-
}
33-
3426
/**
3527
* The allowed keys of the $_FILES array.
3628
*/
@@ -42,6 +34,11 @@ public function __construct(ServerParams $params = null)
4234
'type',
4335
);
4436

37+
public function __construct(ServerParams $params = null)
38+
{
39+
$this->serverParams = $params ?: new ServerParams();
40+
}
41+
4542
/**
4643
* {@inheritdoc}
4744
*/
@@ -121,6 +118,14 @@ public function handleRequest(FormInterface $form, $request = null)
121118
$form->submit($data, 'PATCH' !== $method);
122119
}
123120

121+
public function isFileUpload($data)
122+
{
123+
// POST data will always be strings or arrays of strings. Thus, we can be sure
124+
// that the submitted data is a file upload if the "error" value is an integer
125+
// (this value must have been injected by PHP itself).
126+
return is_array($data) && isset($data['error']) && is_int($data['error']);
127+
}
128+
124129
/**
125130
* Returns the method used to submit the request to the server.
126131
*

‎src/Symfony/Component/Form/RequestHandlerInterface.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,11 @@ interface RequestHandlerInterface
2525
* @param mixed $request The current request
2626
*/
2727
public function handleRequest(FormInterface $form, $request = null);
28+
29+
/**
30+
* @param mixed $data
31+
*
32+
* @return bool
33+
*/
34+
public function isFileUpload($data);
2835
}

0 commit comments

Comments
 (0)