From f23a7f60cf5e18969ce987a594db1b507c6503c4 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Thu, 20 Jun 2019 10:33:16 +0200 Subject: [PATCH] don't validate IP addresses from env var placeholders --- .../DependencyInjection/MainConfiguration.php | 35 --------------- .../DependencyInjection/SecurityExtension.php | 44 +++++++++++++++++-- .../Resources/config/routing.yml | 6 +++ .../SecurityRoutingIntegrationTest.php | 2 +- .../app/StandardFormLogin/config.yml | 5 +++ .../invalid_ip_access_control.yml | 2 +- 6 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index eda62ed9ac71..1e1e97ccb5b3 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -143,15 +143,6 @@ private function addAccessControlSection(ArrayNodeDefinition $rootNode) ->integerNode('port')->defaultNull()->end() ->arrayNode('ips') ->beforeNormalization()->ifString()->then(function ($v) { return [$v]; })->end() - ->beforeNormalization()->always()->then(function ($v) { - foreach ($v as $ip) { - if (false === $this->isValidIp($ip)) { - throw new \LogicException(sprintf('The given "%s" value in the "access_control" config option is not a valid IP address.', $ip)); - } - } - - return $v; - })->end() ->prototype('scalar')->end() ->end() ->arrayNode('methods') @@ -432,30 +423,4 @@ private function addEncodersSection(ArrayNodeDefinition $rootNode) ->end() ; } - - private function isValidIp(string $cidr): bool - { - $cidrParts = explode('/', $cidr); - - if (1 === \count($cidrParts)) { - return false !== filter_var($cidrParts[0], FILTER_VALIDATE_IP); - } - - $ip = $cidrParts[0]; - $netmask = $cidrParts[1]; - - if (!ctype_digit($netmask)) { - return false; - } - - if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { - return $netmask <= 32; - } - - if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { - return $netmask <= 128; - } - - return false; - } } diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index af4260f04c42..4365d0432948 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -731,20 +731,32 @@ private function createExpression($container, $expression) return $this->expressions[$id] = new Reference($id); } - private function createRequestMatcher($container, $path = null, $host = null, int $port = null, $methods = [], $ip = null, array $attributes = []) + private function createRequestMatcher(ContainerBuilder $container, $path = null, $host = null, int $port = null, $methods = [], array $ips = null, array $attributes = []) { if ($methods) { $methods = array_map('strtoupper', (array) $methods); } - $id = '.security.request_matcher.'.ContainerBuilder::hash([$path, $host, $port, $methods, $ip, $attributes]); + if (null !== $ips) { + foreach ($ips as $ip) { + $container->resolveEnvPlaceholders($ip, null, $usedEnvs); + + if (!$usedEnvs && !$this->isValidIp($ip)) { + throw new \LogicException(sprintf('The given value "%s" in the "security.access_control" config option is not a valid IP address.', $ip)); + } + + $usedEnvs = null; + } + } + + $id = '.security.request_matcher.'.ContainerBuilder::hash([$path, $host, $port, $methods, $ips, $attributes]); if (isset($this->requestMatchers[$id])) { return $this->requestMatchers[$id]; } // only add arguments that are necessary - $arguments = [$path, $host, $methods, $ip, $attributes, null, $port]; + $arguments = [$path, $host, $methods, $ips, $attributes, null, $port]; while (\count($arguments) > 0 && !end($arguments)) { array_pop($arguments); } @@ -788,4 +800,30 @@ public function getConfiguration(array $config, ContainerBuilder $container) // first assemble the factories return new MainConfiguration($this->factories, $this->userProviderFactories); } + + private function isValidIp(string $cidr): bool + { + $cidrParts = explode('/', $cidr); + + if (1 === \count($cidrParts)) { + return false !== filter_var($cidrParts[0], FILTER_VALIDATE_IP); + } + + $ip = $cidrParts[0]; + $netmask = $cidrParts[1]; + + if (!ctype_digit($netmask)) { + return false; + } + + if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { + return $netmask <= 32; + } + + if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { + return $netmask <= 128; + } + + return false; + } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml index 2fff93dcad2e..bf82a203d8ad 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml @@ -40,6 +40,12 @@ secured-by-one-real-ip-with-mask: secured-by-one-real-ipv6: path: /secured-by-one-real-ipv6 +secured-by-one-env-placeholder: + path: /secured-by-one-env-placeholder + +secured-by-one-env-placeholder-and-one-real-ip: + path: /secured-by-one-env-placeholder-and-one-real-ip + form_logout: path: /logout_path diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php index 682c561ac5aa..c38bc1a4bf5d 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php @@ -109,7 +109,7 @@ public function testSecurityConfigurationForExpression($config) public function testInvalidIpsInAccessControl() { $this->expectException(\LogicException::class); - $this->expectExceptionMessage('The given "256.357.458.559" value in the "access_control" config option is not a valid IP address.'); + $this->expectExceptionMessage('The given value "256.357.458.559" in the "security.access_control" config option is not a valid IP address.'); $client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'invalid_ip_access_control.yml']); $client->request('GET', '/unprotected_resource'); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml index df03c20c5c51..4e2ac1e11b9d 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml @@ -1,6 +1,9 @@ imports: - { resource: ./../config/default.yml } +parameters: + env(APP_IP): '127.0.0.1' + security: encoders: Symfony\Component\Security\Core\User\User: plaintext @@ -43,6 +46,8 @@ security: - { path: ^/secured-by-one-real-ip$, ips: 198.51.100.0, roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: ^/secured-by-one-real-ip-with-mask$, ips: '203.0.113.0/24', roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: ^/secured-by-one-real-ipv6$, ips: 0:0:0:0:0:ffff:c633:6400, roles: IS_AUTHENTICATED_ANONYMOUSLY } + - { path: ^/secured-by-one-env-placeholder$, ips: '%env(APP_IP)%', roles: IS_AUTHENTICATED_ANONYMOUSLY } + - { path: ^/secured-by-one-env-placeholder-and-one-real-ip$, ips: ['%env(APP_IP)%', 198.51.100.0], roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: ^/highly_protected_resource$, roles: IS_ADMIN } - { path: ^/protected-via-expression$, allow_if: "(is_anonymous() and request.headers.get('user-agent') matches '/Firefox/i') or is_granted('ROLE_USER')" } - { path: .*, roles: IS_AUTHENTICATED_FULLY } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/invalid_ip_access_control.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/invalid_ip_access_control.yml index e4f46c4704af..cc6503affb26 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/invalid_ip_access_control.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/invalid_ip_access_control.yml @@ -19,4 +19,4 @@ security: access_control: # the '256.357.458.559' IP is wrong on purpose, to check invalid IP errors - - { path: ^/unprotected_resource$, ips: [1.1.1.1, 256.357.458.559], roles: IS_AUTHENTICATED_ANONYMOUSLY } + - { path: ^/unprotected_resource$, ips: [1.1.1.1, '%env(APP_IP)%', 256.357.458.559], roles: IS_AUTHENTICATED_ANONYMOUSLY }