8000 bug #23899 [DI] Fix reading env vars from fastcgi params (nicolas-gre… · symfony/symfony@a014222 · GitHub
[go: up one dir, main page]
Skip to content

Commit a014222

Browse files
nicolas-grekasnicolas-grekas
committed
bug #23899 [DI] Fix reading env vars from fastcgi params (nicolas-grekas)
This PR was merged into the 3.3 branch. Discussion ---------- [DI] Fix reading env vars from fastcgi params | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23348 | License | MIT | Doc PR | - Values in fastcgi_param populate `$_SERVER`, never `$_ENV`. This PR makes `$container->getEnv()` read from `$_SERVER`, excluding any vars whose name start by `HTTP_` as that would be a security issue (values injection via HTTP headers.) Embeds a few other fixes found meanwhile. Commits ------- adff65a [DI] Fix reading env vars from fastcgi params
2 parents be7751a + adff65a commit a014222

File tree

5 files changed

+57
-20
lines changed

5 files changed

+57
-20
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,17 @@ protected function processValue($value, $isRoot = false)
5454

5555
$value->setArguments($arguments);
5656

57-
if ($public = $value->isPublic()) {
58-
$value->setPublic(false);
59-
}
6057
$id = 'service_locator.'.md5(serialize($value));
6158

6259
if ($isRoot) {
6360
if ($id !== $this->currentId) {
64-
$this->container->setAlias($id, new Alias($this->currentId, $public));
61+
$this->container->setAlias($id, new Alias($this->currentId, false));
6562
}
6663

6764
return $value;
6865
}
6966

70-
$this->container->setDefinition($id, $value);
67+
$this->container->setDefinition($id, $value->setPublic(false));
7168

7269
return new Reference($id);
7370
}

‎src/Symfony/Component/DependencyInjection/Container.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ public static function underscore($id)
429429
*
430430
* @param string $name The name of the environment variable
431431
*
432-
* @return scalar The value to use for the provided environment variable name
432+
* @return mixed The value to use for the provided environment variable name
433433
*
434434
* @throws EnvNotFoundException When the environment variable is not found and has no default value
435435
*/
@@ -438,6 +438,9 @@ protected function getEnv($name)
438438
if (isset($this->envCache[$name]) || array_key_exists($name, $this->envCache)) {
439439
return $this->envCache[$name];
440440
}
441+
if (0 !== strpos($name, 'HTTP_') && isset($_SERVER[$name])) {
442+
return $this->envCache[$name] = $_SERVER[$name];
443+
}
441444
if (isset($_ENV[$name])) {
442445
return $this->envCache[$name] = $_ENV[$name];
443446
}

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -729,9 +729,10 @@ public function compile(/*$resolveEnvPlaceholders = false*/)
729729
$bag = $this->getParameterBag();
730730

731731
if ($resolveEnvPlaceholders && $bag instanceof EnvPlaceholderParameterBag) {
732-
$this->parameterBag = new ParameterBag($this->resolveEnvPlaceholders($bag->all(), true));
732+
$bag->resolveEnvReferences();
733+
$this->parameterBag = new ParameterBag($bag->all());
733734
$this->envPlaceholders = $bag->getEnvPlaceholders();
734-
$this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($this->parameterBag->all()));
735+
$this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($bag->all(), true));
735736
}
736737

737738
$compiler->compile($this);
@@ -746,7 +747,9 @@ public function compile(/*$resolveEnvPlaceholders = false*/)
746747

747748
parent::compile();
748749

749-
$this->envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : array();
750+
if ($bag instanceof EnvPlaceholderParameterBag) {
751+
$this->envPlaceholders = $bag->getEnvPlaceholders();
752+
}
750753
}
751754

752755
/**
@@ -1311,10 +1314,10 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
13111314
foreach ($envPlaceholders as $env => $placeholders) {
13121315
foreach ($placeholders as $placeholder) {
13131316
if (false !== stripos($value, $placeholder)) {
1314-
if (true === $format) {
1315-
$resolved = $bag->escapeValue($this->getEnv($env));
1316-
} else {
1317+
if (true !== $format) {
13171318
$resolved = sprintf($format, $env);
1319+
} elseif ($placeholder === $resolved = $bag->escapeValue($this->getEnv($env))) {
1320+
$resolved = $bag->all()[strtolower("env($env)")];
13181321
}
13191322
$value = str_ireplace($placeholder, $resolved, $value);
13201323
$usedEnvs[$env] = $env;

‎src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
class EnvPlaceholderParameterBag extends ParameterBag
2121
{
2222
private $envPlaceholders = array();
23+
private $resolveEnvReferences = false;
2324

2425
/**
2526
* {@inheritdoc}
@@ -101,4 +102,29 @@ public function resolve()
101102
}
102103
}
103104
}
105+
106+
/**
107+
* Replaces "%env(FOO)%" references by their placeholder, keeping regular "%parameters%" references as is.
108+
*/
109+
public function resolveEnvReferences()
110+
{
111+
$this->resolveEnvReferences = true;
112+
try {
113+
$this->resolve();
114+
} finally {
115+
$this->resolveEnvReferences = false;
116+
}
117+
}
118+
119+
/**
120+
* {@inheritdoc}
121+
*/
122+
public function resolveString($value, array $resolving = array())
123+
{
124+
if ($this->resolveEnvReferences) {
125+
return preg_replace_callback('/%%|%(env\([^%\s]+\))%/', function ($match) { return isset($match[1]) ? $this->get($match[1]) : '%%'; }, $value);
126+
}
127+
128+
return parent::resolveString($value, $resolving);
129+
}
104130
}

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -603,29 +603,37 @@ public function testMergeThrowsExceptionForDuplicateAutomaticInstanceofDefinitio
603603
public function testResolveEnvValues()
604604
{
605605
$_ENV['DUMMY_ENV_VAR'] = 'du%%y';
606+
$_SERVER['DUMMY_SERVER_VAR'] = 'ABC';
607+
$_SERVER['HTTP_DUMMY_VAR'] = 'DEF';
606608

607609
$container = new ContainerBuilder();
608-
$container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)%');
610+
$container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)% %env(DUMMY_SERVER_VAR)% %env(HTTP_DUMMY_VAR)%');
611+
$container->setParameter('env(HTTP_DUMMY_VAR)', '123');
609612

610-
$this->assertSame('%% du%%%%y', $container->resolveEnvPlaceholders('%bar%', true));
613+
$this->assertSame('%% du%%%%y ABC 123', $container->resolveEnvPlaceholders('%bar%', true));
611614

612-
unset($_ENV['DUMMY_ENV_VAR']);
615+
unset($_ENV['DUMMY_ENV_VAR'], $_SERVER['DUMMY_SERVER_VAR'], $_SERVER['HTTP_DUMMY_VAR']);
613616
}
614617

615618
public function testCompileWithResolveEnv()
616619
{
617-
$_ENV['DUMMY_ENV_VAR'] = 'du%%y';
620+
putenv('DUMMY_ENV_VAR=du%%y');
621+
$_SERVER['DUMMY_SERVER_VAR'] = 'ABC';
622+
$_SERVER['HTTP_DUMMY_VAR'] = 'DEF';
618623

619624
$container = new ContainerBuilder();
620625
$container->setParameter('env(FOO)', 'Foo');
621-
$container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)%');
626+
$container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)% %env(DUMMY_SERVER_VAR)% %env(HTTP_DUMMY_VAR)%');
622627
$container->setParameter('foo', '%env(FOO)%');
628+
$container->setParameter('baz', '%foo%');
629+
$container->setParameter('env(HTTP_DUMMY_VAR)', '123');
623630
$container->compile(true);
624631

625-
$this->assertSame('% du%%y', $container->getParameter('bar'));
626-
$this->assertSame('Foo', $container->getParameter('foo'));
632+
$this->assertSame('% du%%y ABC 123', $container->getParameter('bar'));
633+
$this->assertSame('Foo', $container->getParameter('baz'));
627634

628-
unset($_ENV['DUMMY_ENV_VAR']);
635+
unset($_SERVER['DUMMY_SERVER_VAR'], $_SERVER['HTTP_DUMMY_VAR']);
636+
putenv('DUMMY_ENV_VAR');
629637
}
630638

631639
/**

0 commit comments

Comments
 (0)
0