Skip to content

Commit 81bdc93

Browse files
mbabkermbabker
committed
Harden token unserialization to better align with the security component
1 parent 9fca673 commit 81bdc93

4 files changed

Lines changed: 170 additions & 8 deletions

File tree

‎config/services.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
->args([
103103
service(TokenStorage::class),
104104
abstract_arg('firewalls'),
105+
service(OptionsHandler::class),
105106
])
106107
->call('setLogger', [
107108
service('logger'),

‎src/Authentication/Exception/InvalidTokenException.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace BabDev\WebSocketBundle\Authentication\Exception;
4+
5+
use BabDev\WebSocket\Server\WebSocketException;
6+
7+
final class InvalidTokenException extends \RuntimeException implements WebSocketException {}

‎src/Authentication/Provider/SessionAuthenticationProvider.php‎

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,18 @@
33
namespace BabDev\WebSocketBundle\Authentication\Provider;
44

55
use BabDev\WebSocket\Server\Connection;
6+
use BabDev\WebSocket\Server\IniOptionsHandler;
7+
use BabDev\WebSocket\Server\OptionsHandler;
68
use BabDev\WebSocket\Server\WebSocketException;
79
use BabDev\WebSocketBundle\Authentication\Exception\AuthenticationException;
10+
use BabDev\WebSocketBundle\Authentication\Exception\InvalidTokenException;
811
use BabDev\WebSocketBundle\Authentication\Storage\TokenStorage;
912
use Psr\Log\LoggerAwareInterface;
1013
use Psr\Log\LoggerAwareTrait;
1114
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1215
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1316
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
17+
use Symfony\Component\Security\Core\User\UserInterface;
1418

1519
/**
1620
* The session authentication provider uses the HTTP session for your website's frontend for authenticating to the websocket server.
@@ -29,6 +33,7 @@ final class SessionAuthenticationProvider implements AuthenticationProvider, Log
2933
public function __construct(
3034
private readonly TokenStorage $tokenStorage,
3135
private readonly array $firewalls,
36+
private readonly OptionsHandler $optionsHandler = new IniOptionsHandler(),
3237
) {}
3338

3439
public function supports(Connection $connection): bool
@@ -46,7 +51,6 @@ public function authenticate(Connection $connection): TokenInterface
4651
try {
4752
$token = $this->getToken($connection);
4853
} catch (WebSocketException $exception) {
49-
// Out-of-the-box, we'll get a WebSocketException from our read-only session handler if there was an issue grabbing the session data, so focus only on the component's exceptions
5054
$this->logger?->error('Could not authenticate user.', ['exception' => $exception]);
5155

5256
throw new AuthenticationException('Could not authenticate user.', previous: $exception);
@@ -75,18 +79,78 @@ private function getToken(Connection $connection): TokenInterface
7579
/** @var SessionInterface $session */
7680
$session = $connection->getAttributeStore()->get('session');
7781

82+
$sessionKey = null;
83+
7884
foreach ($this->firewalls as $firewall) {
79-
if (false !== $serializedToken = $session->get('_security_'.$firewall, false)) {
80-
$token = unserialize($serializedToken);
85+
if (false !== $serializedToken = $session->get($sessionKey = '_security_'.$firewall, false)) {
86+
$token = $this->safelyUnserialize($serializedToken, $sessionKey);
87+
88+
$this->logger?->debug('Read existing security token from the session.', [
89+
'key' => $sessionKey,
90+
'token_class' => \is_object($token) ? $token::class : null,
91+
]);
8192

8293
break;
8394
}
8495
}
8596

86-
if (!$token instanceof TokenInterface) {
87-
$token = new NullToken();
97+
if ($token instanceof TokenInterface) {
98+
if (!$token->getUser() instanceof UserInterface) {
99+
throw new InvalidTokenException(\sprintf('Cannot authenticate a "%s" token because it doesn\'t store a user.', $token::class));
100+
}
101+
} elseif (null !== $token) {
102+
$this->logger?->warning('Expected a security token from the session, got something else.', ['key' => $sessionKey, 'received' => $token]);
103+
104+
$token = null;
105+
}
106+
107+
return $token ?? new NullToken();
108+
}
109+
110+
/**
111+
* Safely unserialize a token from the session store.
112+
*
113+
* Forked from {@see \Symfony\Component\Security\Http\Firewall\ContextListener::safelyUnserialize}
114+
*/
115+
private function safelyUnserialize(string $serializedToken, string $sessionKey): mixed
116+
{
117+
$token = null;
118+
119+
$prevUnserializeHandler = $this->optionsHandler->set('unserialize_callback_func', self::class.'::handleUnserializeCallback');
120+
121+
$prevErrorHandler = set_error_handler(static function (int $errno, string $errstr, string $errfile, int $errline) use (&$prevErrorHandler): bool {
122+
if (__FILE__ === $errfile && !\in_array($errno, [\E_DEPRECATED, \E_USER_DEPRECATED], true)) {
123+
throw new \ErrorException($errstr, 0x37313BC, $errno, $errfile, $errline);
124+
}
125+
126+
/** @phpstan-ignore return.type */
127+
return $prevErrorHandler ? $prevErrorHandler($errno, $errstr, $errfile, $errline) : false;
128+
});
129+
130+
try {
131+
$token = unserialize($serializedToken);
132+
} catch (\ErrorException $e) {
133+
if (0x37313BC !== $e->getCode()) {
134+
throw $e;
135+
}
136+
137+
$this->logger?->warning('Failed to unserialize the security token from the session.', ['key' => $sessionKey, 'received' => $serializedToken, 'exception' => $e]);
138+
} finally {
139+
restore_error_handler();
140+
141+
$this->optionsHandler->set('unserialize_callback_func', $prevUnserializeHandler);
88142
}
89143

90144
return $token;
91145
}
146+
147+
/**
148+
* @param class-string $class
149+
*
150+
* @internal
151+
*/
152+
public static function handleUnserializeCallback(string $class): never
153+
{
154+
throw new \ErrorException('Class not found: '.$class, 0x37313BC);
155+
}
92156
}

‎tests/Authentication/Provider/SessionAuthenticationProviderTest.php‎

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use BabDev\WebSocket\Server\Connection;
66
use BabDev\WebSocket\Server\Connection\ArrayAttributeStore;
7+
use BabDev\WebSocketBundle\Authentication\Exception\AuthenticationException;
78
use BabDev\WebSocketBundle\Authentication\Provider\SessionAuthenticationProvider;
89
use BabDev\WebSocketBundle\Authentication\Storage\TokenStorage;
910
use PHPUnit\Framework\MockObject\MockObject;
@@ -125,10 +126,99 @@ public function testAnAuthenticatedUserFromASharedSessionIsAuthenticated(): void
125126
$authenticatedToken = $this->provider->authenticate($connection);
126127

127128
// After https://github.com/symfony/symfony/pull/59558 (introduced in Symfony 7.3), the roleNames property is lazily initialized so we need to trigger that
128-
if (interface_exists(OfflineTokenInterface::class)) {
129-
$authenticatedToken->getRoleNames();
130-
}
129+
$authenticatedToken->getRoleNames();
131130

132131
self::assertEquals($token, $authenticatedToken);
133132
}
133+
134+
public function testANullTokenUsedWhenANonSecurityTokenIsExtractedFromTheSession(): void
135+
{
136+
/** @var MockObject&SessionInterface $session */
137+
$session = $this->createMock(SessionInterface::class);
138+
$session->expects(self::once())
139+
->method('get')
140+
->with('_security_main')
141+
->willReturn(serialize(new \stdClass()));
142+
143+
$attributeStore = new ArrayAttributeStore();
144+
$attributeStore->set('session', $session);
145+
$attributeStore->set('resource_id', 'resource');
146+
147+
/** @var MockObject&Connection $connection */
148+
$connection = $this->createMock(Connection::class);
149+
$connection->method('getAttributeStore')
150+
->willReturn($attributeStore);
151+
152+
$storageIdentifier = '42';
153+
154+
$this->tokenStorage->expects(self::once())
155+
->method('generateStorageId')
156+
->willReturn($storageIdentifier);
157+
158+
$this->tokenStorage->expects(self::once())
159+
->method('addToken')
160+
->with($storageIdentifier, self::isInstanceOf(TokenInterface::class));
161+
162+
self::assertInstanceOf(NullToken::class, $this->provider->authenticate($connection));
163+
}
164+
165+
public function testANullTokenUsedWhenUnserializingTheTokenRaisesAnError(): void
166+
{
167+
/** @var MockObject&SessionInterface $session */
168+
$session = $this->createMock(SessionInterface::class);
169+
$session->expects(self::once())
170+
->method('get')
171+
->with('_security_main')
172+
->willReturn('O:7:"stdClass":0:{}');
173+
174+
$attributeStore = new ArrayAttributeStore();
175+
$attributeStore->set('session', $session);
176+
$attributeStore->set('resource_id', 'resource');
177+
178+
/** @var MockObject&Connection $connection */
179+
$connection = $this->createMock(Connection::class);
180+
$connection->method('getAttributeStore')
181+
->willReturn($attributeStore);
182+
183+
$storageIdentifier = '42';
184+
185+
$this->tokenStorage->expects(self::once())
186+
->method('generateStorageId')
187+
->willReturn($storageIdentifier);
188+
189+
$this->tokenStorage->expects(self::once())
190+
->method('addToken')
191+
->with($storageIdentifier, self::isInstanceOf(TokenInterface::class));
192+
193+
self::assertInstanceOf(NullToken::class, $this->provider->authenticate($connection));
194+
}
195+
196+
public function testDoesNotAuthenticateWhenATokenWithoutAUserIsUnserialized(): void
197+
{
198+
/** @var MockObject&SessionInterface $session */
199+
$session = $this->createMock(SessionInterface::class);
200+
$session->expects(self::once())
201+
->method('get')
202+
->with('_security_main')
203+
->willReturn(serialize(new NullToken()));
204+
205+
$attributeStore = new ArrayAttributeStore();
206+
$attributeStore->set('session', $session);
207+
$attributeStore->set('resource_id', 'resource');
208+
209+
/** @var MockObject&Connection $connection */
210+
$connection = $this->createMock(Connection::class);
211+
$connection->method('getAttributeStore')
212+
->willReturn($attributeStore);
213+
214+
$this->tokenStorage->expects(self::never())
215+
->method('generateStorageId');
216+
217+
$this->tokenStorage->expects(self::never())
218+
->method('addToken');
219+
220+
$this->expectException(AuthenticationException::class);
221+
222+
$this->provider->authenticate($connection);
223+
}
134224
}

0 commit comments

Comments
 (0)