Skip to content

Commit 956437c

Browse files
committed
fix(core): bound the compact serializers' segment split
The compact JWS and JWE serializers split the whole input on every "." before checking the segment count, so a delimiter-heavy string was first expanded into one array entry per delimiter. That costs about 25 times the size of the input in memory: a 2 MB token allocates ~48 MB before it is rejected as malformed. The split is now bounded to one more segment than a valid token has, which is enough to detect and reject longer input while keeping the allocation proportional to the token itself. The accepted and rejected inputs are unchanged. Reported by Team Atlanta.
1 parent cf4d1d3 commit 956437c

4 files changed

Lines changed: 118 additions & 2 deletions

File tree

src/Library/Encryption/Serializer/CompactSerializer.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,16 @@ public function serialize(JWE $jwe, ?int $recipientIndex = null): string
5454
);
5555
}
5656

57+
/**
58+
* The split is bounded to six segments: a valid compact JWE has exactly five, so a sixth one is enough to
59+
* detect and reject any longer input. Without that bound, a delimiter-heavy string would be expanded into one
60+
* array entry per delimiter before the segment count is checked, which costs about twenty-five times the size
61+
* of the input in memory.
62+
*/
5763
#[Override]
5864
public function unserialize(string $input): JWE
5965
{
60-
$parts = explode('.', $input);
66+
$parts = explode('.', $input, 6);
6167
if (count($parts) !== 5) {
6268
throw new InvalidArgumentException('Unsupported input');
6369
}

src/Library/Signature/Serializer/CompactSerializer.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,16 @@ public function serialize(JWS $jws, ?int $signatureIndex = null): string
5858
);
5959
}
6060

61+
/**
62+
* The split is bounded to four segments: a valid compact JWS has exactly three, so a fourth one is enough to
63+
* detect and reject any longer input. Without that bound, a delimiter-heavy string would be expanded into one
64+
* array entry per delimiter before the segment count is checked, which costs about twenty-five times the size
65+
* of the input in memory.
66+
*/
6167
#[Override]
6268
public function unserialize(string $input): JWS
6369
{
64-
$parts = explode('.', $input);
70+
$parts = explode('.', $input, 4);
6571
if (count($parts) !== 3) {
6672
throw new InvalidArgumentException('Unsupported input');
6773
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Jose\Tests\Component\Encryption;
6+
7+
use InvalidArgumentException;
8+
use Jose\Component\Encryption\Serializer\CompactSerializer;
9+
use PHPUnit\Framework\Attributes\Test;
10+
use PHPUnit\Framework\TestCase;
11+
use function memory_get_peak_usage;
12+
use function memory_get_usage;
13+
use function memory_reset_peak_usage;
14+
use function str_repeat;
15+
16+
/**
17+
* @internal
18+
*/
19+
final class CompactSerializerTest extends TestCase
20+
{
21+
#[Test]
22+
public function aTokenWithTooManySegmentsIsRejected(): void
23+
{
24+
$this->expectException(InvalidArgumentException::class);
25+
$this->expectExceptionMessage('Unsupported input');
26+
27+
(new CompactSerializer())->unserialize('eyJhbGciOiJkaXIiLCJlbmMiOiJBMTI4R0NNIn0.....');
28+
}
29+
30+
/**
31+
* A delimiter-heavy token must be rejected without expanding it into one array entry per delimiter. The
32+
* threshold is deliberately generous: the unbounded split of this input allocates roughly twenty-five times its
33+
* size, while the bounded one stays proportional to it.
34+
*/
35+
#[Test]
36+
public function aDelimiterHeavyTokenIsRejectedWithoutExhaustingMemory(): void
37+
{
38+
$token = str_repeat('.', 2_000_000);
39+
$serializer = new CompactSerializer();
40+
memory_reset_peak_usage();
41+
$before = memory_get_usage();
42+
43+
try {
44+
$serializer->unserialize($token);
45+
static::fail('The token should have been rejected.');
46+
} catch (InvalidArgumentException $e) {
47+
static::assertSame('Unsupported input', $e->getMessage());
48+
}
49+
50+
static::assertLessThan(8_000_000, memory_get_peak_usage() - $before);
51+
}
52+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Jose\Tests\Component\Signature;
6+
7+
use InvalidArgumentException;
8+
use Jose\Component\Signature\Serializer\CompactSerializer;
9+
use PHPUnit\Framework\Attributes\Test;
10+
use PHPUnit\Framework\TestCase;
11+
use function memory_get_peak_usage;
12+
use function memory_get_usage;
13+
use function memory_reset_peak_usage;
14+
use function str_repeat;
15+
16+
/**
17+
* @internal
18+
*/
19+
final class CompactSerializerTest extends TestCase
20+
{
21+
#[Test]
22+
public function aTokenWithTooManySegmentsIsRejected(): void
23+
{
24+
$this->expectException(InvalidArgumentException::class);
25+
$this->expectExceptionMessage('Unsupported input');
26+
27+
(new CompactSerializer())->unserialize('eyJhbGciOiJub25lIn0...');
28+
}
29+
30+
/**
31+
* A delimiter-heavy token must be rejected without expanding it into one array entry per delimiter. The
32+
* threshold is deliberately generous: the unbounded split of this input allocates roughly twenty-five times its
33+
* size, while the bounded one stays proportional to it.
34+
*/
35+
#[Test]
36+
public function aDelimiterHeavyTokenIsRejectedWithoutExhaustingMemory(): void
37+
{
38+
$token = str_repeat('.', 2_000_000);
39+
$serializer = new CompactSerializer();
40+
memory_reset_peak_usage();
41+
$before = memory_get_usage();
42+
43+
try {
44+
$serializer->unserialize($token);
45+
static::fail('The token should have been rejected.');
46+
} catch (InvalidArgumentException $e) {
47+
static::assertSame('Unsupported input', $e->getMessage());
48+
}
49+
50+
static::assertLessThan(8_000_000, memory_get_peak_usage() - $before);
51+
}
52+
}

0 commit comments

Comments
 (0)