Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3876c75

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
security #cve-2019-10910 [DI] Check service IDs are valid (nicolas-grekas)
This PR was merged into the 2.8 branch. Discussion ---------- [DI] Check service IDs are valid Based on #87 Commits ------- 7fdfeefdfb [DI] Check service IDs are valid
1 parent 41e9ec3 commit 3876c75
Copy full SHA for 3876c75

File tree

4 files changed

+60
-12
lines changed
Filter options

4 files changed

+60
-12
lines changed

‎src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,17 @@ public function isProxyCandidate(Definition $definition)
5353
public function getProxyFactoryCode(Definition $definition, $id)
5454
{
5555
$instantiation = 'return';
56+
$scope = '';
5657

5758
if ($definition->isShared()) {
58-
$instantiation .= " \$this->services['$id'] =";
59+
$instantiation .= ' $this->services[%s] =';
5960

6061
if (\defined('Symfony\Component\DependencyInjection\ContainerInterface::SCOPE_CONTAINER') && ContainerInterface::SCOPE_CONTAINER !== $scope = $definition->getScope(false)) {
61-
$instantiation .= " \$this->scopedServices['$scope']['$id'] =";
62+
$instantiation .= ' $this->scopedServices[%s][%1$s] =';
6263
}
6364
}
6465

66+
$instantiation = sprintf($instantiation, var_export($id, true), var_export($scope, true));
6567
$methodName = 'get'.Container::camelize($id).'Service';
6668
$proxyClass = $this->getProxyClassName($definition);
6769

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,10 @@ public function setAlias($alias, $id)
641641
{
642642
$alias = strtolower($alias);
643643

644+
if ('' === $alias || '\\' === substr($alias, -1) || \strlen($alias) !== strcspn($alias, "\0\r\n'")) {
645+
throw new InvalidArgumentException(sprintf('Invalid alias id: "%s"', $alias));
646+
}
647+
644648
if (\is_string($id)) {
645649
$id = new Alias($id);
646650
} elseif (!$id instanceof Alias) {
@@ -775,6 +779,10 @@ public function setDefinition($id, Definition $definition)
775779

776780
$id = strtolower($id);
777781

782+
if ('' === $id || '\\' === substr($id, -1) || \strlen($id) !== strcspn($id, "\0\r\n'")) {
783+
throw new InvalidArgumentException(sprintf('Invalid service id: "%s"', $id));
784+
}
785+
778786
unset($this->aliasDefinitions[$id]);
779787

780788
return $this->definitions[$id] = $definition;

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+16-10Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -379,9 +379,9 @@ private function addServiceInstance($id, Definition $definition)
379379
$instantiation = '';
380380

381381
if (!$isProxyCandidate && $definition->isShared() && ContainerInterface::SCOPE_CONTAINER === $definition->getScope(false)) {
382-
$instantiation = "\$this->services['$id'] = ".($simple ? '' : '$instance');
382+
$instantiation = sprintf('$this->services[%s] = %s', var_export($id, true), $simple ? '' : '$instance');
383383
} elseif (!$isProxyCandidate && $definition->isShared() && ContainerInterface::SCOPE_PROTOTYPE !== $scope = $definition->getScope(false)) {
384-
$instantiation = "\$this->services['$id'] = \$this->scopedServices['$scope']['$id'] = ".($simple ? '' : '$instance');
384+
$instantiation = sprintf('$this->services[%s] = $this->scopedServices[%s][%1$s] = %s', var_export($id, true), var_export($scope, true), $simple ? '' : '$instance');
385385
} elseif (!$simple) {
386386
$instantiation = '$instance';
387387
}
@@ -609,6 +609,9 @@ private function addService($id, Definition $definition)
609609
* Gets the $public '$id'$shared$autowired service.
610610
*
611611
* $return
612+
EOF;
613+
$code = str_replace('*/', ' ', $code).<<<EOF
614+
612615
*/
613616
{$visibility} function get{$this->camelize($id)}Service($lazyInitialization)
614617
{
@@ -620,15 +623,15 @@ private function addService($id, Definition $definition)
620623
if (!\in_array($scope, array(ContainerInterface::SCOPE_CONTAINER, ContainerInterface::SCOPE_PROTOTYPE))) {
621624
$code .= <<<EOF
622625
if (!isset(\$this->scopedServices['$scope'])) {
623-
throw new InactiveScopeException('$id', '$scope');
626+
throw new InactiveScopeException({$this->export($id)}, '$scope');
624627
}
625628
626629
627630
EOF;
628631
}
629632

630633
if ($definition->isSynthetic()) {
631-
$code .= sprintf(" throw new RuntimeException('You have requested a synthetic service (\"%s\"). The DIC does not know how to construct this service.');\n }\n", $id);
634+
$code .= sprintf(" throw new RuntimeException(%s);\n }\n", var_export("You have requested a synthetic service (\"$id\"). The DIC does not know how to construct this service.", true));
632635
} else {
633636
if ($definition->isDeprecated()) {
634637
$code .= sprintf(" @trigger_error(%s, E_USER_DEPRECATED);\n\n", var_export($definition->getDeprecationMessage($id), true));
@@ -706,10 +709,11 @@ private function addServiceSynchronizer($id, Definition $definition)
706709
$arguments[] = $this->dumpValue($value);
707710
}
708711

709-
$call = $this->wrapServiceConditionals($call[1], sprintf("\$this->get('%s')->%s(%s);", $definitionId, $call[0], implode(', ', $arguments)));
712+
$definitionId = var_export($definitionId, true);
713+
$call = $this->wrapServiceConditionals($call[1], sprintf('$this->get(%s)->%s(%s);', $definitionId, $call[0], implode(', ', $arguments)));
710714

711715
$code .= <<<EOF
712-
if (\$this->initialized('$definitionId')) {
716+
if (\$this->initialized($definitionId)) {
713717
$call
714718
}
715719
@@ -1142,7 +1146,7 @@ private function wrapServiceConditionals($value, $code)
11421146

11431147
$conditions = array();
11441148
foreach ($services as $service) {
1145-
$conditions[] = sprintf("\$this->has('%s')", $service);
1149+
$conditions[] = sprintf('$this->has(%s)', var_export($service, true));
11461150
}
11471151

11481152
// re-indent the wrapped code
@@ -1419,11 +1423,13 @@ private function dumpLiteralClass($class)
14191423
*/
14201424
public function dumpParameter($name)
14211425
{
1426+
$name = (string) $name;
1427+
14221428
if ($this->container->isFrozen() && $this->container->hasParameter($name)) {
14231429
return $this->dumpValue($this->container->getParameter($name), false);
14241430
}
14251431

1426-
return sprintf("\$this->getParameter('%s')", strtolower($name));
1432+
return sprintf('$this->getParameter(%s)', var_export($name, true));
14271433
}
14281434

14291435
/**
@@ -1458,10 +1464,10 @@ private function getServiceCall($id, Reference $reference = null)
14581464
}
14591465

14601466
if (null !== $reference && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $reference->getInvalidBehavior()) {
1461-
return sprintf('$this->get(\'%s\', ContainerInterface::NULL_ON_INVALID_REFERENCE)', $id);
1467+
return sprintf('$this->get(%s, ContainerInterface::NULL_ON_INVALID_REFERENCE)', var_export($id, true));
14621468
}
14631469

1464-
return sprintf('$this->get(\'%s\')', $id);
1470+
return sprintf('$this->get(%s)', var_export($id, true));
14651471
}
14661472

14671473
/**

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
+32Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,38 @@ public function testNonSharedServicesReturnsDifferentInstances()
131131
$this->assertNotSame($builder->get('bar'), $builder->get('bar'));
132132
}
133133

134+
/**
135+
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
136+
* @dataProvider provideBadId
137+
*/
138+
public function testBadAliasId($id)
139+
{
140+
$builder = new ContainerBuilder();
141+
$builder->setAlias($id, 'foo');
142+
}
143+
144+
/**
145+
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
146+
* @dataProvider provideBadId
147+
*/
148+
public function testBadDefinitionId($id)
149+
{
150+
$builder = new ContainerBuilder();
151+
$builder->setDefinition($id, new Definition('Foo'));
152+
}
153+
154+
public function provideBadId()
155+
{
156+
return [
157+
[''],
158+
["\0"],
159+
["\r"],
160+
["\n"],
161+
["'"],
162+
['ab\\'],
163+
];
164+
}
165+
134166
/**
135167
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
136168
* @expectedExceptionMessage You have requested a synthetic service ("foo"). The DIC does not know how to construct this service.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.