8000 security #cve-2019-10910 [DI] Check service IDs are valid (nicolas-gr… · donquixote/symfony@789a34a · GitHub
[go: up one dir, main page]
Skip to content

Commit 789a34a

Browse files
nicolas-grekasnicolas-grekas
committed
security #cve-2019-10910 [DI] Check service IDs are valid (nicolas-grekas)
This PR was merged into the 2.7 branch. Discussion ---------- [DI] Check service IDs are valid Based on symfony#87 Commits ------- 0671884f41 [DI] Check service IDs are valid
1 parent 783ef2f commit 789a34a

File tree

4 files changed

+61
-14
lines changed

4 files changed

+61
-14
lines changed

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,13 @@ public function getProxyFactoryCode(Definition $definition, $id)
5454
{
5555
$instantiation = 'return';
5656

57-
if (ContainerInterface::SCOPE_CONTAINER === $definition->getScope()) {
58-
$instantiation .= " \$this->services['$id'] =";
59-
} elseif (ContainerInterface::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) {
60-
$instantiation .= " \$this->services['$id'] = \$this->scopedServices['$scope']['$id'] =";
57+
if (ContainerInterface::SCOPE_CONTAINER === $scope = $definition->getScope()) {
58+
$instantiation .= ' $this->services[%s] =';
59+
} elseif (ContainerInterface::SCOPE_PROTOTYPE !== $definition->getScope()) {
60+
$instantiation .= ' $this->services[%s] = $this->scopedServices[%s][%1$s] =';
6161
}
6262

63+
$instantiation = sprintf($instantiation, var_export($id, true), var_export($scope, true));
6364
$methodName = 'get'.Container::camelize($id).'Service';
6465
$proxyClass = $this->getProxyClassName($definition);
6566

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,10 @@ public function setAlias($alias, $id)
622622
{
623623
$alias = strtolower($alias);
624624

625+
if ('' === $alias || '\\' === substr($alias, -1) || \strlen($alias) !== strcspn($alias, "\0\r\n'")) {
626+
throw new InvalidArgumentException(sprintf('Invalid alias id: "%s"', $alias));
627+
}
628+
625629
if (is_string($id)) {
626630
$id = new Alias($id);
627631
} elseif (!$id instanceof Alias) {
@@ -756,6 +760,10 @@ public function setDefinition($id, Definition $definition)
756760

757761
$id = strtolower($id);
758762

763+
if ('' === $id || '\\' === substr($id, -1) || \strlen($id) !== strcspn($id, "\0\r\n'")) {
764+
throw new InvalidArgumentException(sprintf('Invalid service id: "%s"', $id));
765+
}
766+
759767
unset($this->aliasDefinitions[$id]);
760768

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

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -377,9 +377,9 @@ private function addServiceInstance($id, Definition $definition)
377377
$instantiation = '';
378378

379379
if (!$isProxyCandidate && ContainerInterface::SCOPE_CONTAINER === $definition->getScope()) {
380-
$instantiation = "\$this->services['$id'] = ".($simple ? '' : '$instance');
380+
$instantiation = sprintf('$this->services[%s] = %s', var_export($id, true), $simple ? '' : '$instance');
381381
} elseif (!$isProxyCandidate && ContainerInterface::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) {
382-
$instantiation = "\$this->services['$id'] = \$this->scopedServices['$scope']['$id'] = ".($simple ? '' : '$instance');
382+
$instantiation = sprintf('$this->services[%s] = $this->scopedServices[%s][%1$s] = %s', var_export($id, true), var_export($scope, true), $simple ? '' : '$instance');
383383
} elseif (!$simple) {
384384
$instantiation = '$instance';
385385
}
@@ -598,6 +598,9 @@ private function addService($id, Definition $definition)
598598
* Gets the $public '$id'$shared service.
599599
*
600600
* $return
601+
EOF;
602+
$code = str_replace('*/', ' ', $code).<<<EOF
603+
601604
*/
602605
{$visibility} function get{$this->camelize($id)}Service($lazyInitialization)
603606
{
@@ -609,15 +612,15 @@ private function addService($id, Definition $definition)
609612
if (!in_array($scope, array(ContainerInterface::SCOPE_CONTAINER, ContainerInterface::SCOPE_PROTOTYPE))) {
610613
$code .= <<<EOF
611614
if (!isset(\$this->scopedServices['$scope'])) {
612-
throw new InactiveScopeException('$id', '$scope');
615+
throw new InactiveScopeException({$this->export($id)}, '$scope');
613616
}
614617
615618
616619
EOF;
617620
}
618621

619622
if ($definition->isSynthetic()) {
620-
$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);
623+
$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));
621624
} else {
622625
$code .=
623626
$this->addServiceInclude($definition).
@@ -691,10 +694,11 @@ private function addServiceSynchronizer($id, Definition $definition)
691694
$arguments[] = $this->dumpValue($value);
692695
}
693696

694-
$call = $this->wrapServiceConditionals($call[1], sprintf("\$this->get('%s')->%s(%s);", $definitionId, $call[0], implode(', ', $arguments)));
697+
$definitionId = var_export($definitionId, true);
698+
$call = $this->wrapServiceConditionals($call[1], sprintf('$this->get(%s)->%s(%s);', $definitionId, $call[0], implode(', ', $arguments)));
695699

696700
$code .= <<<EOF
697-
if (\$this->initialized('$definitionId')) {
701+
if (\$this->initialized($definitionId)) {
698702
$call
699703
}
700704
@@ -1127,7 +1131,7 @@ private function wrapServiceConditionals($value, $code)
11271131

11281132
$conditions = array();
11291133
foreach ($services as $service) {
1130-
$conditions[] = sprintf("\$this->has('%s')", $service);
1134+
$conditions[] = sprintf('$this->has(%s)', var_export($service, true));
11311135
}
11321136

11331137
// re-indent the wrapped code
@@ -1404,11 +1408,13 @@ private function dumpLiteralClass($class)
14041408
*/
14051409
public function dumpParameter($name)
14061410
{
1411+
$name = (string) $name;
1412+
14071413
if ($this->container->isFrozen() && $this->container->hasParameter($name)) {
14081414
return $this->dumpValue($this->container->getParameter($name), false);
14091415
}
14101416

1411-
return sprintf("\$this->getParameter('%s')", strtolower($name));
1417+
return sprintf('$this->getParameter(%s)', var_export($name, true));
14121418
}
14131419

14141420
/**
@@ -1443,10 +1449,10 @@ private function getServiceCall($id, Reference $reference = null)
14431449
}
14441450

14451451
if (null !== $reference && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $reference->getInvalidBehavior()) {
1446-
return sprintf('$this->get(\'%s\', ContainerInterface::NULL_ON_INVALID_REFERENCE)', $id);
1452+
return sprintf('$this->get(%s, ContainerInterface::NULL_ON_INVALID_REFERENCE)', var_export($id, true));
14471453
}
14481454

1449-
return sprintf('$this->get(\'%s\')', $id);
1455+
return sprintf('$this->get(%s)', var_export($id, true));
14501456
}
14511457

14521458
/**

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,38 @@ public function testGet()
111111
$this->assertSame($builder->get('bar'), $builder->get('bar'), '->get() always returns the same instance if the service is shared');
112112
}
113113

114+
/**
115+
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
116+
* @dataProvider provideBadId
117+
*/
118+
public function testBadAliasId($id)
119+
{
120+
$builder = new ContainerBuilder();
121+
$builder->setAlias($id, 'foo');
122+
}
123+
124+
/**
125+
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
126+
* @dataProvider provideBadId
127+
*/
128+
public function testBadDefinitionId($id)
129+
{
130+
$builder = new ContainerBuilder();
131+
$builder->setDefinition($id, new Definition('Foo'));
132+
}
133+
134+
public function provideBadId()
135+
{
136+
return [
137+
[''],
138+
["\0"],
139+
["\r"],
140+
["\n"],
141+
["'"],
142+
['ab\\'],
143+
];
144+
}
145+
114146
/**
115147
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
116148
* @expectedExceptionMessage You have requested a synthetic service ("foo"). The DIC does not know how to construct this service.

0 commit comments

Comments
 (0)
0