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

[WebProfilerBundle] [VarDumper] Inject "unsafe-eval" into the CSP if the VarDumper was used #29155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[WebProfilerBundle] [VarDumper] Inject "unsafe-eval" into the CSP if the VarDumper was used #29155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions 42 src/Symfony/Bundle/WebProfilerBundle/Csp/ContentSecurityPolicyHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class ContentSecurityPolicyHandler
{
private $nonceGenerator;
private $cspDisabled = false;
private $evaluationEnabled = false;

public function __construct(NonceGenerator $nonceGenerator)
{
Expand Down Expand Up @@ -92,10 +93,25 @@ public function updateResponseHeaders(Request $request, Response $response): arr
$nonces = $this->getNonces($request, $response);
$this->cleanHeaders($response);
$this->updateCspHeaders($response, $nonces);
$this->setEvaluationEnabled(false);

return $nonces;
}

public function isEvaluationEnabled(): bool
{
return $this->evaluationEnabled;
}

/**
* Allows/disallows the evaluation of code via functions like eval().
* If enabled 'unsafe-eval' will be set in the Content-Security-Policy.
*/
public function setEvaluationEnabled(bool $enabled): void
{
$this->evaluationEnabled = $enabled;
}

private function cleanHeaders(Response $response)
{
$response->headers->remove('X-SymfonyProfiler-Script-Nonce');
Expand Down Expand Up @@ -142,6 +158,11 @@ private function updateCspHeaders(Response $response, array $nonces = []): array
}
$headers[$header][$type][] = sprintf('\'nonce-%s\'', $nonces[$tokenName]);
}

if ($this->evaluationEnabled && !$this->authorizesEval($directives, 'script-src')) {
$ruleIsSet = true;
$headers[$header]['script-src'][] = '\'unsafe-eval\'';
}
}

if (!$ruleIsSet) {
Expand Down Expand Up @@ -208,6 +229,27 @@ private function authorizesInline(array $directivesSet, string $type): bool
return \in_array('\'unsafe-inline\'', $directives, true) && !$this->hasHashOrNonce($directives);
}

/**
* Detects if the 'unsafe-eval' is prevented for a directive within the directive set.
*
* @param array $directivesSet The directive set
* @param string $type The name of the directive to check
*
* @return bool
*/
private function authorizesEval(array $directivesSet, $type)
{
if (isset($directivesSet[$type])) {
$directives = $directivesSet[$type];
} elseif (isset($directivesSet['default-src'])) {
$directives = $directivesSet['default-src'];
} else {
return false;
}

return \in_array('\'unsafe-eval\'', $directives, true);
}

private function hasHashOrNonce(array $directives): bool
{
foreach ($directives as $directive) {
Expand Down
10 changes: 10 additions & 0 deletions 10 src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\VarDumper\VarDumper;
use Twig\Environment;

/**
Expand Down Expand Up @@ -111,6 +112,14 @@ public function onKernelResponse(ResponseEvent $event)
$this->injectToolbar($response, $request, $nonces);
}

public function onKernelRequest(): void
{
herndlm marked this conversation as resolved.
Show resolved Hide resolved
// Because as of now the dumper embeds scripts they are evaluated in base_js.html.twig
VarDumper::setAfterDumpHandlerOnce(function () {
$this->cspHandler->setEvaluationEnabled(true);
});
}

/**
* Injects the web debug toolbar into the given Response.
*/
Expand Down Expand Up @@ -139,6 +148,7 @@ public static function getSubscribedEvents(): array
{
return [
KernelEvents::RESPONSE => ['onKernelResponse', -128],
KernelEvents::REQUEST => ['onKernelRequest'],
];
}
}
88 changes: 88 additions & 0 deletions 88 src/Symfony/Bundle/WebProfilerBundle/Tests/Csp/ContentSecurityPolicyHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,25 @@ public function testOnKernelResponse($nonce, $expectedNonce, Request $request, R
}
}

/**
* @dataProvider provideRequestAndResponsesForOnKernelResponseEvaluation
*/
public function testOnKernelResponseEvaluation($nonce, $expectedNonce, Request $request, Response $response, array $expectedCsp): void
{
$cspHandler = new ContentSecurityPolicyHandler($this->mockNonceGenerator($nonce));

$cspHandler->setEvaluationEnabled(true);
$this->assertSame($expectedNonce, $cspHandler->updateResponseHeaders($request, $response));

$this->assertFalse($cspHandler->isEvaluationEnabled(), 'evaluationEnabled has to be disabled again');
$this->assertFalse($response->headers->has('X-SymfonyProfiler-Script-Nonce'));
$this->assertFalse($response->headers->has('X-SymfonyProfiler-Style-Nonce'));

foreach ($expectedCsp as $header => $value) {
$this->assertSame($value, $response->headers->get($header));
}
}

public function provideRequestAndResponses()
{
$nonce = bin2hex(random_bytes(16));
Expand Down Expand Up @@ -178,6 +197,75 @@ public function provideRequestAndResponsesForOnKernelResponse()
];
}

/**
* @throws \Exception
*/
public function provideRequestAndResponsesForOnKernelResponseEvaluation(): array
{
$nonce = bin2hex(random_bytes(16));

return [
// Cases where nothing needs to be done because evaluation is already enabled
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(),
['Content-Security-Policy' => null, 'Content-Security-Policy-Report-Only' => null, 'X-Content-Security-Policy' => null],
],
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(['Content-Security-Policy' => 'default-src \'self\' \'unsafe-inline\' \'unsafe-eval\'']),
['Content-Security-Policy' => 'default-src \'self\' \'unsafe-inline\' \'unsafe-eval\'', 'X-Content-Security-Policy' => null],
],
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(['Content-Security-Policy' => 'default-src \'self\'; script-src \'unsafe-inline\' \'unsafe-eval\'; style-src \'unsafe-inline\'']),
['Content-Security-Policy' => 'default-src \'self\'; script-src \'unsafe-inline\' \'unsafe-eval\'; style-src \'unsafe-inline\'', 'X-Content-Security-Policy' => null],
],
// Cases where evaluation needs to be enabled
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(['Content-Security-Policy' => 'default-src \'self\' \'unsafe-inline\'']),
['Content-Security-Policy' => 'default-src \'self\' \'unsafe-inline\'; script-src \'unsafe-eval\'', 'X-Content-Security-Policy' => null],
],
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(['Content-Security-Policy' => 'script-src \'self\' \'unsafe-inline\'']),
['Content-Security-Policy' => 'script-src \'self\' \'unsafe-inline\' \'unsafe-eval\'', 'X-Content-Security-Policy' => null],
],
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(['Content-Security-Policy' => 'default-src \'unsafe-inline\'; script-src \'self\' \'unsafe-inline\'']),
['Content-Security-Policy' => 'default-src \'unsafe-inline\'; script-src \'self\' \'unsafe-inline\' \'unsafe-eval\'', 'X-Content-Security-Policy' => null],
],
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(['Content-Security-Policy' => 'default-src \'unsafe-inline\'; script-src \'self\' \'unsafe-inline\'']),
['Content-Security-Policy' => 'default-src \'unsafe-inline\'; script-src \'self\' \'unsafe-inline\' \'unsafe-eval\'', 'X-Content-Security-Policy' => null],
],
[
$nonce,
['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
$this->createRequest(),
$this->createResponse(['Content-Security-Policy' => 'default-src \'none\'; script-src \'self\' \'unsafe-inline\'; style-src \'unsafe-inline\'']),
['Content-Security-Policy' => 'default-src \'none\'; script-src \'self\' \'unsafe-inline\' \'unsafe-eval\'; style-src \'unsafe-inline\'', 'X-Content-Security-Policy' => null],
],
];
}

private function createRequest(array $headers = [])
{
$request = new Request();
Expand Down
30 changes: 29 additions & 1 deletion 30 src/Symfony/Component/VarDumper/Tests/Dumper/FunctionsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,40 @@ public function testDumpReturnsAllArgsInArray()
$this->assertEquals([$var1, $var2, $var3], $return);
}

protected function setupVarDumper()
public function testAfterDumpHandler()
{
$handlerCalled = false;
$this->setupVarDumper(function () use (&$handlerCalled) {
$handlerCalled = true;
});

$var1 = 'a';

ob_start();
$return = dump($var1);
$out = ob_get_clean();

$this->assertTrue($handlerCalled);
$this->assertEquals($var1, $return);

// Test if the afterDumpHandlerOnce is only being used once
$handlerCalled = false;

ob_start();
$return = dump($var1);
$out = ob_get_clean();

$this->assertFalse($handlerCalled, 'The afterDumpHandlerOnce must only be used once');
$this->assertEquals($var1, $return);
}

protected function setupVarDumper(callable $afterDumpHandlerOnce = null)
{
$cloner = new VarCloner();
$dumper = new CliDumper('php://output');
VarDumper::setHandler(function ($var) use ($cloner, $dumper) {
$dumper->dump($cloner->cloneVar($var));
});
VarDumper::setAfterDumpHandlerOnce($afterDumpHandlerOnce);
}
}
18 changes: 17 additions & 1 deletion 18 src/Symfony/Component/VarDumper/VarDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
class VarDumper
{
private static $handler;
private static $afterDumpHandlerOnce;

public static function dump($var)
{
Expand All @@ -47,7 +48,14 @@ public static function dump($var)
};
}

return (self::$handler)($var);
$output = (self::$handler)($var);

if (null !== self::$afterDumpHandlerOnce) {
(self::$afterDumpHandlerOnce)($var);
self::$afterDumpHandlerOnce = null;
}

return $output;
}

public static function setHandler(callable $callable = null)
Expand All @@ -57,4 +65,12 @@ public static function setHandler(callable $callable = null)

return $prevHandler;
}

public static function setAfterDumpHandlerOnce(callable $callable = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like something we'd better avoid IMHO.
I think it's fine if we call setEvaluationEnabled(true) all the time when dump() is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for getting back in touch so late. I'm aware this a not-important edge-case and I actually needed this at my old company and currently I don't. Anyways but I still want to finish this :)

Do you mean to adapt this callback to be called every-time instead of that only once logic which simplifies it or something completely different?

{
$prevHandler = self::$afterDumpHandlerOnce;
self::$afterDumpHandlerOnce = $callable;

return $prevHandler;
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.