-
Notifications
You must be signed in to change notification settings - Fork 2k
refactor: restrict object deserialization in selected unserialize() calls #10254
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,7 +120,7 @@ public function get(CLIRequest|IncomingRequest $request, ResponseInterface $resp | |
| $cachedResponse = $this->cache->get($this->generateCacheKey($request)); | ||
|
|
||
| if (is_string($cachedResponse) && $cachedResponse !== '') { | ||
| $cachedResponse = unserialize($cachedResponse); | ||
| $cachedResponse = unserialize($cachedResponse, ['allowed_classes' => false]); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| if ( | ||
| ! is_array($cachedResponse) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ class ArrayCast extends BaseCast | |
| public static function get($value, array $params = []): array | ||
| { | ||
| if (is_string($value) && (str_starts_with($value, 'a:') || str_starts_with($value, 's:'))) { | ||
| $value = unserialize($value); | ||
| $value = unserialize($value, ['allowed_classes' => false]); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may break existing applications that store arrays containing entities or other objects. This is why it wasn't changed. |
||
| } | ||
|
|
||
| return (array) $value; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cached arrays may legitimately contain nested objects. This changes existing behavior by restoring them as
__PHP_Incomplete_Class. It also does not prevent cache poisoning because an attacker with direct Redis write access can set__ci_type = object.