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 #87

Commits
-------

0671884f41 [DI] Check service IDs are valid
This commit is contained in:
Nicolas Grekas 2019-04-16 12:48:27 +02:00
parent 783ef2fb1d
commit 789a34ad76
4 changed files with 61 additions and 14 deletions

View File

@ -54,12 +54,13 @@ class ProxyDumper implements DumperInterface
{
$instantiation = 'return';
if (ContainerInterface::SCOPE_CONTAINER === $definition->getScope()) {
$instantiation .= " \$this->services['$id'] =";
} elseif (ContainerInterface::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) {
$instantiation .= " \$this->services['$id'] = \$this->scopedServices['$scope']['$id'] =";
if (ContainerInterface::SCOPE_CONTAINER === $scope = $definition->getScope()) {
$instantiation .= ' $this->services[%s] =';
} elseif (ContainerInterface::SCOPE_PROTOTYPE !== $definition->getScope()) {
$instantiation .= ' $this->services[%s] = $this->scopedServices[%s][%1$s] =';
}
$instantiation = sprintf($instantiation, var_export($id, true), var_export($scope, true));
$methodName = 'get'.Container::camelize($id).'Service';
$proxyClass = $this->getProxyClassName($definition);

View File

@ -622,6 +622,10 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
{
$alias = strtolower($alias);
if ('' === $alias || '\\' === substr($alias, -1) || \strlen($alias) !== strcspn($alias, "\0\r\n'")) {
throw new InvalidArgumentException(sprintf('Invalid alias id: "%s"', $alias));
}
if (is_string($id)) {
$id = new Alias($id);
} elseif (!$id instanceof Alias) {
@ -756,6 +760,10 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
$id = strtolower($id);
if ('' === $id || '\\' === substr($id, -1) || \strlen($id) !== strcspn($id, "\0\r\n'")) {
throw new InvalidArgumentException(sprintf('Invalid service id: "%s"', $id));
}
unset($this->aliasDefinitions[$id]);
return $this->definitions[$id] = $definition;

View File

@ -377,9 +377,9 @@ class PhpDumper extends Dumper
$instantiation = '';
if (!$isProxyCandidate && ContainerInterface::SCOPE_CONTAINER === $definition->getScope()) {
$instantiation = "\$this->services['$id'] = ".($simple ? '' : '$instance');
$instantiation = sprintf('$this->services[%s] = %s', var_export($id, true), $simple ? '' : '$instance');
} elseif (!$isProxyCandidate && ContainerInterface::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) {
$instantiation = "\$this->services['$id'] = \$this->scopedServices['$scope']['$id'] = ".($simple ? '' : '$instance');
$instantiation = sprintf('$this->services[%s] = $this->scopedServices[%s][%1$s] = %s', var_export($id, true), var_export($scope, true), $simple ? '' : '$instance');
} elseif (!$simple) {
$instantiation = '$instance';
}
@ -598,6 +598,9 @@ class PhpDumper extends Dumper
* Gets the $public '$id'$shared service.
*
* $return
EOF;
$code = str_replace('*/', ' ', $code).<<<EOF
*/
{$visibility} function get{$this->camelize($id)}Service($lazyInitialization)
{
@ -609,7 +612,7 @@ EOF;
if (!in_array($scope, array(ContainerInterface::SCOPE_CONTAINER, ContainerInterface::SCOPE_PROTOTYPE))) {
$code .= <<<EOF
if (!isset(\$this->scopedServices['$scope'])) {
throw new InactiveScopeException('$id', '$scope');
throw new InactiveScopeException({$this->export($id)}, '$scope');
}
@ -617,7 +620,7 @@ EOF;
}
if ($definition->isSynthetic()) {
$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);
$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));
} else {
$code .=
$this->addServiceInclude($definition).
@ -691,10 +694,11 @@ EOF;
$arguments[] = $this->dumpValue($value);
}
$call = $this->wrapServiceConditionals($call[1], sprintf("\$this->get('%s')->%s(%s);", $definitionId, $call[0], implode(', ', $arguments)));
$definitionId = var_export($definitionId, true);
$call = $this->wrapServiceConditionals($call[1], sprintf('$this->get(%s)->%s(%s);', $definitionId, $call[0], implode(', ', $arguments)));
$code .= <<<EOF
if (\$this->initialized('$definitionId')) {
if (\$this->initialized($definitionId)) {
$call
}
@ -1127,7 +1131,7 @@ EOF;
$conditions = array();
foreach ($services as $service) {
$conditions[] = sprintf("\$this->has('%s')", $service);
$conditions[] = sprintf('$this->has(%s)', var_export($service, true));
}
// re-indent the wrapped code
@ -1404,11 +1408,13 @@ EOF;
*/
public function dumpParameter($name)
{
$name = (string) $name;
if ($this->container->isFrozen() && $this->container->hasParameter($name)) {
return $this->dumpValue($this->container->getParameter($name), false);
}
return sprintf("\$this->getParameter('%s')", strtolower($name));
return sprintf('$this->getParameter(%s)', var_export($name, true));
}
/**
@ -1443,10 +1449,10 @@ EOF;
}
if (null !== $reference && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $reference->getInvalidBehavior()) {
return sprintf('$this->get(\'%s\', ContainerInterface::NULL_ON_INVALID_REFERENCE)', $id);
return sprintf('$this->get(%s, ContainerInterface::NULL_ON_INVALID_REFERENCE)', var_export($id, true));
}
return sprintf('$this->get(\'%s\')', $id);
return sprintf('$this->get(%s)', var_export($id, true));
}
/**

View File

@ -111,6 +111,38 @@ class ContainerBuilderTest extends TestCase
$this->assertSame($builder->get('bar'), $builder->get('bar'), '->get() always returns the same instance if the service is shared');
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @dataProvider provideBadId
*/
public function testBadAliasId($id)
{
$builder = new ContainerBuilder();
$builder->setAlias($id, 'foo');
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @dataProvider provideBadId
*/
public function testBadDefinitionId($id)
{
$builder = new ContainerBuilder();
$builder->setDefinition($id, new Definition('Foo'));
}
public function provideBadId()
{
return [
[''],
["\0"],
["\r"],
["\n"],
["'"],
['ab\\'],
];
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage You have requested a synthetic service ("foo"). The DIC does not know how to construct this service.