# Patch Analysis ## Patch/fix summary The ticket describes several unsafe deserialization and command-injection issues fixed before/in Grav CMS 2.0.0-beta.2. Source inspection compared `2.0.0-beta.1` (`26a2d519c59c620e2b0a54d0baf33889d7d5db0a`) with `2.0.0-beta.2` (`f95b0ff51a655edcbcc060a3d74b43e3f20b9585`). The same fixed tag was also the latest tag available in the prepared repository. ### `system/src/Grav/Common/Scheduler/JobQueue.php` - Vulnerable behavior: `reconstructJob()` trusted `serialized_job` and executed `unserialize(base64_decode(...))` directly. - Fixed behavior: `push()` writes a sibling `serialized_job_hmac = hash_hmac('sha256', $serialized, Security::getNonceKey())`. `reconstructJob()` only unserializes `serialized_job` when both `serialized_job` and `serialized_job_hmac` are present, base64 decoding succeeds, and `hash_equals()` verifies the expected HMAC. - Fallback behavior: If the HMAC is absent, mismatched, or invalid, the code falls through to reconstructing a job from structured fields (`command`, `arguments`, `job_id`) instead of trusting the serialized object. ### `system/src/Grav/Framework/Cache/Adapter/FileCache.php` - Vulnerable behavior: cache files stored `expires\nkey\nserialize(value)` and `doGet()` returned `unserialize($value, ['allowed_classes' => true])` when the key matched. - Fixed behavior: cache files now use a versioned envelope: `v2\n\n\n\n`. `doGet()` rejects non-`v2` files, stale entries, key mismatches, and HMAC mismatches before calling `unserialize()`. - Rejected files are treated as cache misses and removed, causing clean rebuild. ### `system/src/Grav/Common/Session.php` - Vulnerable behavior: `getFlashObject()` accepted a bare serialized string from session storage and called `unserialize($serialized, ['allowed_classes' => true])`. - Fixed behavior: `setFlashObject()` writes `v2||` and `getFlashObject()` only unserializes when the HMAC verifies with `Security::getNonceKey()`. Unsigned legacy strings and forged HMAC values return `null` instead of being unserialized. ### `system/src/Grav/Console/Cli/InstallCommand.php` - Vulnerable behavior: `gitclone()` concatenated `.dependencies` values into a shell command: `git clone -b --depth 1 `. - Fixed behavior: `branch`, `url`, and `path` are passed through `escapeshellarg()`, and `--` is inserted before `url` and `path` to block git option injection. ## Fix assumptions The deserialization fix assumes that the attacker can plant or tamper with persistent files but cannot compute a valid HMAC under Grav's per-site `Security::getNonceKey()`. This is a reasonable trust boundary: the HMAC key is server-side and intended to be unavailable to unauthenticated remote/API attackers. The fix also assumes that same-site code or an actor with direct access to the private HMAC key is already trusted enough to create legitimate serialized queue/cache/session payloads. The variant probe confirmed that a correct HMAC still reaches `unserialize()` on 2.0.0-beta.2; this is expected behavior, not a bypass, because the probe signs the payload inside the local Grav process. For command injection, the fix assumes that shell metacharacter injection is prevented by quoting all untrusted argument positions and that git option injection is prevented by the `--` separator before repository/path arguments. ## Code paths/inputs covered Covered by the fix: - Unsigned `Scheduler\JobQueue` `serialized_job` fields. - Tampered or forged-HMAC `Scheduler\JobQueue` `serialized_job` fields. - Legacy/pre-v2 `FileCache` cache files. - Tampered, stale-key, malformed, or mismatched-HMAC `FileCache` cache files. - Legacy/bare `Session::getFlashObject()` payloads. - Forged-HMAC `Session::getFlashObject()` payloads. - Shell metacharacters in `.dependencies` `branch`, `url`, and `path` fields in `InstallCommand::gitclone()`. ## Code paths/inputs not covered Not covered by design: - Serialized queue/cache/session payloads that are signed with the legitimate site HMAC key. - Same-process PHP code that calls `Security::getNonceKey()` and creates signed malicious payloads. - Attackers who already have local filesystem/code execution sufficient to read or invoke the private HMAC key. No distinct in-scope path was found where a remote/API attacker can supply a malformed queue/cache/session payload to 2.0.0-beta.2 and still reach unsafe `unserialize()` without satisfying the HMAC invariant. ## Behavior before and after the fix Before (`2.0.0-beta.1`): - `JobQueue::reconstructJob()` accepted and unserialized any base64 `serialized_job` field. - `FileCache::doGet()` accepted pre-v2 cache files and unserialized the remaining file contents after an expiry/key check. - `Session::getFlashObject()` accepted any bare serialized string stored in the session slot. - `.dependencies` values were concatenated into a shell command in `InstallCommand::gitclone()`. After (`2.0.0-beta.2`): - `JobQueue`, `FileCache`, and `Session` enforce versioned/HMAC integrity before `unserialize()`. - Unsigned/tampered serialized data is rejected or safely reconstructed from structured fields. - `InstallCommand::gitclone()` quotes untrusted shell arguments and separates git options from operands. ## Threat model / security policy scope `SECURITY.md` defines critical issues as no-account cases that can modify content or run malicious code without access. The parent reproduction crosses an API/remote trust boundary by triggering scheduler processing through the real scheduler-webhook endpoint. The variant candidate that worked on 2.0.0-beta.2 does not meet this scope because it needs access to `Security::getNonceKey()` in the same local site context. That precondition is closer to local/same-process control than unauthenticated remote exploitation. ## Variant conclusion The fixed version appears complete for the in-scope unsafe-deserialization trust boundary tested here. The only fixed-version execution observed is a same-site signed payload, which is expected legitimate behavior and not a bypass. No distinct validated bypass should be handed to coding as a vulnerability; recommended hardening is to maintain HMAC integrity and add regression coverage for unsigned/forged/malformed inputs across all serialized payload sinks.