Skip to content

🔒 Replace serialize/unserialize with JSON for route cache#33

Open
dimitriBouteille wants to merge 3 commits intodevelopfrom
fix/cache-unserialize
Open

🔒 Replace serialize/unserialize with JSON for route cache#33
dimitriBouteille wants to merge 3 commits intodevelopfrom
fix/cache-unserialize

Conversation

@dimitriBouteille
Copy link
Copy Markdown
Owner

Summary

Replace serialize() / unserialize() with JSON for the PSR-6 route cache, and fix the unreachable try/catch block that left RouteLoader::getRoutes() returning false when the cache payload was corrupted.

Why

RouteLoader::getRoutes() deserialized the cached route list with unserialize():

try {
    return unserialize($cacheRoutes->get());
} catch (\Exception) {
}

Two problems:

  1. PHP object injection risk. If the PSR-6 backend (Redis, Memcached, file cache, shared opcache…) is poisoned, an attacker can inject a serialized payload that triggers a POP gadget chain. The cached data only contains plain
    strings (class names, paths, HTTP methods) — there is no need to allow arbitrary class deserialization.
  2. Dead catch block. unserialize() does not throw exceptions; on failure it returns false. The catch never fires, so a corrupted cache leaked through as false, and register() then crashed on foreach ($routes as $route).

What changed

  • RouteLoader::getRoutes() now persists routes as JSON and validates the shape on read. Any anomaly (invalid JSON, missing/wrong-typed fields, unsupported callback) triggers a transparent fallback to findRoutes() and overwrites the cache.
  • New hydrateRoutes(string): ?Route[] rebuilds Route / RouteAction instances and returns null whenever the payload deviates from the expected shape — letting the caller rebuild instead of returning a half-broken list.
  • New dehydrateRoutes(Route[]): ?string flattens routes into plain arrays, uses JSON_THROW_ON_ERROR, and refuses to cache when a permissionCallback cannot be safely serialized (Closure, arbitrary object). Such routes still
    register normally; only the cache write is skipped, with an E_USER_NOTICE so it shows up in dev.
  • New isCacheableCallback(mixed): bool accepts only null, strings (class-string / function name) and [class-string, method-string] arrays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant