CVE-2026-42608
Description
Grav is a file-based Web platform. Prior to 2.0.0-beta.2, there is a Path Traversal vulnerability within the FormFlash core component. By manipulating the session_id (passed as __form-flash-id in POST requests), an unauthenticated attacker can traverse the filesystem to create arbitrary directories and write an index.yaml file containing attacker-controlled data. This vulnerability can lead to unauthorized modification of application behavior, potential data integrity issues, and service disruption in production environments. This vulnerability is fixed in 2.0.0-beta.2.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
getgrav/gravPackagist | < 2.0.0-beta.2 | 2.0.0-beta.2 |
Affected products
1Patches
2c66dfeb5ff67Security fixes: cache + serialization integrity, git-clone shell escape
9 files changed · +524 −33
CHANGELOG.md+2 −0 modified@@ -13,6 +13,8 @@ * [security] Fixed unauthenticated path traversal in `FormFlash` (GHSA-hmcx-ch82-3fv2): `session_id` and `unique_id` now pass through a strict allowlist before being used to build on-disk paths, preventing arbitrary directory creation via the `__form-flash-id` parameter. * [security] Fixed salt disclosure via sandboxed Twig (GHSA-3f29-pqwf-v4j4): the HMAC key used for CSRF nonces and admin rate-limit hashing has moved out of Config into `user/config/security-private.php` (blocked from web access by the default `user/*.php` htaccess rule), so `{{ grav.config.get('security.salt') }}` no longer leaks it. Existing sites are migrated automatically on first request — existing nonces and sessions survive the upgrade. * [security] Hardened the new-user uniqueness guard in `UserObject::save` (GHSA-rr73-568v-28f8): `strpos(..., '@@')` replaced with `str_contains` (the old form was falsy when the transient-key marker was at position 0, silently bypassing the check) and the check now runs for every `FlexStorageInterface` backend rather than only `FileStorage`. A low-privileged user with `admin.users.create` can no longer disrupt a super-admin account by submitting the admin's username through the "add user" form. + * [security] Added HMAC integrity to `Framework\Cache\Adapter\FileCache` (GHSA-gwfr-jfjf-92vv): every cache payload is signed with `Security::getNonceKey()` on write and verified on read; tampered, attacker-planted, or pre-upgrade files are treated as cache misses and removed instead of being unserialized. The on-disk format is now versioned (`v2\n<expires>\n<key>\n<hmac>\n<serialized>`); existing caches rebuild transparently on first read. + * [security] Closed GHSA-vj3m-2g9h-vm4p (5-part advisory): (#1) `JobQueue` now HMAC-signs the `serialized_job` blob — a tampered queue file can no longer smuggle a forged `Job` for direct RCE via `Job::exec → call_user_func_array`; legitimate items still execute via the structured-fields fallback. (#3) `Session::getFlashObject` now wraps its payload in a versioned HMAC envelope, refusing to unserialize legacy/forged values. (#4) `InstallCommand` git-clone arguments (`branch`, `url`, `path` from `user/.dependencies`) now go through `escapeshellarg`, with a `--` separator before url/path to block option-injection. (#5) `twig_array_reduce` (plus `twig_array_some`/`twig_array_every`) added to `cleanDangerousTwig`'s callable blocklist alongside the existing `twig_array_map`/`filter`. (#2) was the same FileCache issue addressed by GHSA-gwfr above. # v2.0.0-beta.1 ## 04/16/2026
system/src/Grav/Common/Scheduler/JobQueue.php+30 −13 modified@@ -9,6 +9,7 @@ namespace Grav\Common\Scheduler; +use Grav\Common\Security; use RocketTheme\Toolbox\File\JsonFile; use RuntimeException; @@ -91,8 +92,13 @@ public function push(Job $job, string $priority = self::PRIORITY_NORMAL): string 'metadata' => [], ]; - // Always serialize the job to preserve its full state - $queueItem['serialized_job'] = base64_encode(serialize($job)); + // Always serialize the job to preserve its full state. The blob is HMAC-signed + // with the per-site nonce key so that a tampered queue file cannot be used to + // smuggle a forged Job (which would gain RCE via Job::exec → call_user_func_array). + // GHSA-vj3m-2g9h-vm4p (#1). + $serialized = serialize($job); + $queueItem['serialized_job'] = base64_encode($serialized); + $queueItem['serialized_job_hmac'] = hash_hmac('sha256', $serialized, Security::getNonceKey()); $this->writeQueueItem($queueItem, 'pending'); @@ -459,26 +465,37 @@ protected function getItemsInDirectory(string $directory): array */ protected function reconstructJob(array $item): ?Job { - if (isset($item['serialized_job'])) { - // Unserialize the job - try { - $job = unserialize(base64_decode((string) $item['serialized_job'])); - if ($job instanceof Job) { - return $job; + // GHSA-vj3m-2g9h-vm4p (#1): refuse to unserialize a queue item whose + // HMAC is missing or doesn't match — a tampered or attacker-planted + // queue file could otherwise inject a forged Job for direct RCE. + if (isset($item['serialized_job'], $item['serialized_job_hmac'])) { + $serialized = base64_decode((string) $item['serialized_job'], true); + if ($serialized !== false) { + $expected = hash_hmac('sha256', $serialized, Security::getNonceKey()); + if (hash_equals((string) $item['serialized_job_hmac'], $expected)) { + try { + $job = unserialize($serialized, ['allowed_classes' => true]); + if ($job instanceof Job) { + return $job; + } + } catch (\Exception) { + return null; + } } - } catch (\Exception) { - // Failed to unserialize - return null; } + // HMAC missing/mismatched/decode failed — fall through to the + // structured-fields rebuild below so legitimate queue items + // written before this fix still run, but without trusting their + // serialized state. } - + // Create a new job from command if (isset($item['command'])) { $args = $item['arguments'] ?? []; $job = new Job($item['command'], $args, $item['job_id']); return $job; } - + return null; }
system/src/Grav/Common/Security.php+6 −2 modified@@ -283,8 +283,12 @@ public static function getXssDefaults(): array * Property access (e.g. {{ page.header }}) is allowed; calls (header(...), obj.header(...), |header) are blocked. */ private const CALLABLE_DANGEROUS_NAMES = [ - // Twig internals - 'twig_array_map', 'twig_array_filter', 'call_user_func', 'call_user_func_array', + // Twig internals — every callback-taking helper. GHSA-vj3m-2g9h-vm4p (#5) + // called out the missing `twig_array_reduce`; adding the other Twig 3 + // callback predicates (some/every) at the same time as defense-in-depth. + 'twig_array_map', 'twig_array_filter', 'twig_array_reduce', + 'twig_array_some', 'twig_array_every', + 'call_user_func', 'call_user_func_array', 'forward_static_call', 'forward_static_call_array', // Twig environment manipulation 'registerUndefinedFunctionCallback', 'registerUndefinedFilterCallback',
system/src/Grav/Common/Session.php+27 −4 modified@@ -97,7 +97,11 @@ public function started() */ public function setFlashObject($name, mixed $object) { - $this->__set($name, serialize($object)); + // GHSA-vj3m-2g9h-vm4p (#3): wrap the serialized payload with an HMAC so a + // tampered session file can't smuggle in arbitrary class instantiation. + $serialized = serialize($object); + $hmac = hash_hmac('sha256', $serialized, Security::getNonceKey()); + $this->__set($name, "v2|{$hmac}|" . $serialized); return $this; } @@ -110,9 +114,28 @@ public function setFlashObject($name, mixed $object) */ public function getFlashObject($name) { - $serialized = $this->__get($name); - - $object = is_string($serialized) ? unserialize($serialized, ['allowed_classes' => true]) : $serialized; + $stored = $this->__get($name); + + $object = null; + if (is_string($stored) && str_starts_with($stored, 'v2|')) { + // 3-field format: v2|<hmac>|<serialized>. The serialized payload may + // itself contain `|`, so split with limit=3. + $parts = explode('|', $stored, 3); + if (count($parts) === 3) { + [, $expectedHmac, $serialized] = $parts; + $actualHmac = hash_hmac('sha256', $serialized, Security::getNonceKey()); + if (hash_equals($expectedHmac, $actualHmac)) { + try { + $object = unserialize($serialized, ['allowed_classes' => true]); + } catch (\Throwable) { + $object = null; + } + } + } + } elseif (!is_string($stored)) { + $object = $stored; + } + // Legacy unsigned strings or HMAC mismatches fall through with $object = null. $this->__unset($name);
system/src/Grav/Console/Cli/InstallCommand.php+13 −1 modified@@ -147,7 +147,19 @@ private function gitclone(): int foreach ($this->config['git'] as $repo => $data) { $path = $this->destination . DS . $data['path']; if (!file_exists($path)) { - exec('cd ' . escapeshellarg($this->destination) . ' && git clone -b ' . $data['branch'] . ' --depth 1 ' . $data['url'] . ' ' . $data['path'], $output, $return); + // GHSA-vj3m-2g9h-vm4p (#4): branch/url/path come from user/.dependencies + // and must be shell-escaped before reaching exec() — otherwise a planted + // .dependencies file gains command injection when an admin runs install. + // The bare `--` blocks option-injection in url/path positions + // (e.g. a `path` value like `--upload-pack=evil`). + $cmd = sprintf( + 'cd %s && git clone -b %s --depth 1 -- %s %s', + escapeshellarg($this->destination), + escapeshellarg((string) $data['branch']), + escapeshellarg((string) $data['url']), + escapeshellarg((string) $data['path']) + ); + exec($cmd, $output, $return); if (!$return) { $io->writeln('<green>SUCCESS</green> cloned <magenta>' . $data['url'] . '</magenta> -> <cyan>' . $path . '</cyan>');
system/src/Grav/Framework/Cache/Adapter/FileCache.php+46 −13 modified@@ -11,6 +11,7 @@ use ErrorException; use FilesystemIterator; +use Grav\Common\Security; use Grav\Framework\Cache\AbstractCache; use Grav\Framework\Cache\Exception\CacheException; use Grav\Framework\Cache\Exception\InvalidArgumentException; @@ -24,10 +25,20 @@ * * Defaults to 1 year TTL. Does not support unlimited TTL. * + * Cache files are HMAC-signed (sha256, key from Security::getNonceKey()) so that a + * tampered or attacker-planted file is rejected as a cache miss instead of + * being unserialized — closes GHSA-gwfr-jfjf-92vv. The on-disk format is: + * + * v2\n<expires>\n<key>\n<hmac-hex>\n<serialized> + * + * Pre-v2 files (no version line) are treated as cache misses and rebuilt. + * * @package Grav\Framework\Cache */ class FileCache extends AbstractCache { + private const FORMAT_VERSION = 'v2'; + /** @var string */ private $directory; /** @var string|null */ @@ -63,20 +74,38 @@ public function doGet($key, $miss) return $miss; } - if ($now >= (int) $expiresAt = fgets($h)) { + $version = rtrim((string)fgets($h)); + if ($version !== self::FORMAT_VERSION) { + // Pre-v2 file (or junk). Drop it; the caller will repopulate. fclose($h); @unlink($file); - } else { - $i = rawurldecode(rtrim((string)fgets($h))); - $value = stream_get_contents($h) ?: ''; + return $miss; + } + + if ($now >= (int) $expiresAt = fgets($h)) { fclose($h); + @unlink($file); + return $miss; + } - if ($i === $key) { - return unserialize($value, ['allowed_classes' => true]); - } + $i = rawurldecode(rtrim((string)fgets($h))); + $expectedHmac = rtrim((string)fgets($h)); + $value = stream_get_contents($h) ?: ''; + fclose($h); + + if ($i !== $key) { + return $miss; } - return $miss; + $actualHmac = hash_hmac('sha256', $value, Security::getNonceKey()); + if (!hash_equals($expectedHmac, $actualHmac)) { + // Tampered or stale-secret payload — refuse to unserialize and + // delete the file so it gets rebuilt cleanly next time. + @unlink($file); + return $miss; + } + + return unserialize($value, ['allowed_classes' => true]); } /** @@ -86,12 +115,16 @@ public function doGet($key, $miss) public function doSet($key, $value, $ttl) { $expiresAt = time() + (int)$ttl; + $serialized = serialize($value); + $hmac = hash_hmac('sha256', $serialized, Security::getNonceKey()); + + $payload = self::FORMAT_VERSION . "\n" + . $expiresAt . "\n" + . rawurlencode($key) . "\n" + . $hmac . "\n" + . $serialized; - $result = $this->write( - $this->getFile($key, true), - $expiresAt . "\n" . rawurlencode($key) . "\n" . serialize($value), - $expiresAt - ); + $result = $this->write($this->getFile($key, true), $payload, $expiresAt); if (!$result && !is_writable($this->directory)) { throw new CacheException(sprintf('Cache directory is not writable (%s)', $this->directory));
tests/unit/Grav/Common/Security/CleanDangerousTwigTest.php+5 −0 modified@@ -366,6 +366,11 @@ public static function providerCallbackFunctions(): array ['{{ array_map("system", ["id"]) }}', 'array_map with callback'], ['{{ array_filter(arr, "system") }}', 'array_filter with callback'], ['{{ usort(arr, "system") }}', 'usort with callback'], + // GHSA-vj3m-2g9h-vm4p (#5): twig_array_reduce was missing from the + // blocklist alongside its already-listed twig_array_map/filter siblings. + ['{{ twig_array_reduce(arr, "system", "") }}', 'twig_array_reduce GHSA-vj3m'], + ['{{ twig_array_some(arr, "system") }}', 'twig_array_some'], + ['{{ twig_array_every(arr, "system") }}', 'twig_array_every'], ]; }
tests/unit/Grav/Common/Security/FileCacheSecurityTest.php+163 −0 added@@ -0,0 +1,163 @@ +<?php + +use Codeception\Util\Fixtures; +use Grav\Common\Grav; +use Grav\Common\Security; +use Grav\Framework\Cache\Adapter\FileCache; + +/** + * Class FileCacheSecurityTest + * + * Covers: GHSA-gwfr-jfjf-92vv (insecure deserialization in FileCache). + * + * Verifies the HMAC-integrity wrapper around FileCache's on-disk payloads: + * - round-trip with the same key works, + * - a tampered payload is treated as a cache miss and the file is removed, + * - a pre-v2 file (no version line) is treated as a cache miss and removed, + * - a payload signed with a different key is rejected. + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class FileCacheSecurityTest extends \PHPUnit\Framework\TestCase +{ + /** @var Grav */ + protected $grav; + + /** @var string */ + protected $cacheRoot; + + protected function setUp(): void + { + parent::setUp(); + $grav = Fixtures::get('grav'); + $this->grav = $grav(); + + $this->cacheRoot = sys_get_temp_dir() . '/grav-filecache-sec-' . bin2hex(random_bytes(4)); + @mkdir($this->cacheRoot, 0777, true); + } + + protected function tearDown(): void + { + $this->rrmdir($this->cacheRoot); + parent::tearDown(); + } + + private function rrmdir(string $dir): void + { + if (!is_dir($dir)) { + return; + } + foreach (scandir($dir) as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + $path = "{$dir}/{$entry}"; + is_dir($path) ? $this->rrmdir($path) : @unlink($path); + } + @rmdir($dir); + } + + private function newCache(): FileCache + { + return new FileCache('test', 60, $this->cacheRoot); + } + + private function findCacheFile(): ?string + { + $iter = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($this->cacheRoot, FilesystemIterator::SKIP_DOTS)); + foreach ($iter as $file) { + if ($file->isFile()) { + return (string)$file; + } + } + return null; + } + + // ========================================================================= + // GHSA-gwfr-jfjf-92vv: HMAC integrity on file cache payloads + // ========================================================================= + + public function testGetSet_GHSAgwfr_RoundTripPreservesValue(): void + { + $cache = $this->newCache(); + $cache->set('alpha', ['hello' => 'world', 'n' => 42]); + + self::assertSame(['hello' => 'world', 'n' => 42], $cache->get('alpha')); + } + + public function testGet_GHSAgwfr_RejectsTamperedPayload(): void + { + $cache = $this->newCache(); + $cache->set('alpha', 'original-value'); + + $file = $this->findCacheFile(); + self::assertNotNull($file, 'cache file should exist after set'); + + // Flip a byte inside the serialized payload (last segment after the 4 + // header lines: v2, expires, key, hmac). + $contents = file_get_contents($file); + $lines = explode("\n", $contents, 5); + self::assertCount(5, $lines, 'cache file must have 5 segments'); + $lines[4] = str_replace('original-value', 'taintednvalue', $lines[4]); + file_put_contents($file, implode("\n", $lines)); + + $miss = '__MISS__'; + self::assertSame($miss, $cache->get('alpha', $miss), 'tampered file must be a miss'); + self::assertFileDoesNotExist($file, 'tampered file must be deleted'); + } + + public function testGet_GHSAgwfr_RejectsForgedHmacWithDifferentKey(): void + { + $cache = $this->newCache(); + $file = $cache->set('alpha', 'real-value'); + + // Reuse the file path. Hand-craft a payload whose HMAC was computed + // with the wrong key — exactly what an attacker who can write to the + // cache directory but cannot read user/config/security-private.php + // would produce. + $file = $this->findCacheFile(); + $serialized = serialize('attacker-payload'); + $forgedHmac = hash_hmac('sha256', $serialized, 'wrong-key-attacker-guessed'); + $payload = "v2\n" . (time() + 60) . "\n" . rawurlencode('alpha') . "\n" . $forgedHmac . "\n" . $serialized; + file_put_contents($file, $payload); + + $miss = '__MISS__'; + self::assertSame($miss, $cache->get('alpha', $miss), 'forged HMAC must be a miss'); + self::assertFileDoesNotExist($file, 'forged file must be deleted'); + } + + public function testGet_GHSAgwfr_RejectsPreV2FormatFile(): void + { + // Mimic the legacy file format: <expires>\n<key>\n<serialized> + // No version line, no HMAC. Pre-upgrade caches end up here. + $cache = $this->newCache(); + $cache->set('alpha', 'placeholder'); // create the file so getFile() path exists + + $file = $this->findCacheFile(); + self::assertNotNull($file); + $legacy = (time() + 60) . "\n" . rawurlencode('alpha') . "\n" . serialize('legacy-value'); + file_put_contents($file, $legacy); + + $miss = '__MISS__'; + self::assertSame($miss, $cache->get('alpha', $miss), 'pre-v2 file must be a miss'); + self::assertFileDoesNotExist($file, 'pre-v2 file must be deleted'); + } + + public function testGet_GHSAgwfr_RejectsKeyMismatchInPayload(): void + { + // The on-disk key field is part of the existing collision check; a + // valid HMAC over a payload whose key field doesn't match what we + // asked for must NOT be returned to the caller. + $cache = $this->newCache(); + $cache->set('alpha', 'a-value'); + $file = $this->findCacheFile(); + + $serialized = serialize('a-value'); + $hmac = hash_hmac('sha256', $serialized, Security::getNonceKey()); + $payload = "v2\n" . (time() + 60) . "\n" . rawurlencode('beta') . "\n" . $hmac . "\n" . $serialized; + file_put_contents($file, $payload); + + $miss = '__MISS__'; + self::assertSame($miss, $cache->get('alpha', $miss), 'key-field mismatch must be a miss'); + } +}
tests/unit/Grav/Common/Security/UnserializeIntegritySecurityTest.php+232 −0 added@@ -0,0 +1,232 @@ +<?php + +use Codeception\Util\Fixtures; +use Grav\Common\Grav; +use Grav\Common\Security; +use Grav\Common\Scheduler\Job; +use Grav\Common\Scheduler\JobQueue; + +/** + * Class UnserializeIntegritySecurityTest + * + * Covers: GHSA-vj3m-2g9h-vm4p (#1 JobQueue, #3 Session) — HMAC integrity + * around `unserialize(..., ['allowed_classes' => true])` sinks so that a + * tampered payload cannot smuggle in arbitrary class instantiation. + * + * Note: the FileCache half of this advisory (#2) has its own dedicated + * test in FileCacheSecurityTest. + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class UnserializeIntegritySecurityTest extends \PHPUnit\Framework\TestCase +{ + /** @var Grav */ + protected $grav; + + /** @var string */ + protected $queueDir; + + protected function setUp(): void + { + parent::setUp(); + $grav = Fixtures::get('grav'); + $this->grav = $grav(); + + $this->queueDir = sys_get_temp_dir() . '/grav-jobqueue-sec-' . bin2hex(random_bytes(4)); + @mkdir($this->queueDir, 0777, true); + } + + protected function tearDown(): void + { + if (is_dir($this->queueDir)) { + $iter = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($this->queueDir, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($iter as $f) { + $f->isDir() ? @rmdir((string)$f) : @unlink((string)$f); + } + @rmdir($this->queueDir); + } + parent::tearDown(); + } + + // ========================================================================= + // GHSA-vj3m-2g9h-vm4p (#1): JobQueue serialized_job HMAC integrity + // ========================================================================= + + /** + * Drive `JobQueue::reconstructJob` with a hand-built queue item; the + * method is protected, so step through it with reflection. + */ + private function reconstruct(JobQueue $queue, array $item): ?Job + { + $m = (new ReflectionClass($queue))->getMethod('reconstructJob'); + $m->setAccessible(true); + return $m->invoke($queue, $item); + } + + public function testReconstructJob_GHSAvj3m_RoundTripsValidHmacSignedJob(): void + { + $queue = new JobQueue($this->queueDir); + + $job = new Job('echo', ['ok'], 'job-1'); + $serialized = serialize($job); + $item = [ + 'serialized_job' => base64_encode($serialized), + 'serialized_job_hmac' => hash_hmac('sha256', $serialized, Security::getNonceKey()), + 'job_id' => 'job-1', + ]; + + $reconstructed = $this->reconstruct($queue, $item); + self::assertInstanceOf(Job::class, $reconstructed); + self::assertSame('job-1', $reconstructed->getId()); + } + + public function testReconstructJob_GHSAvj3m_RejectsForgedSerializedJob(): void + { + $queue = new JobQueue($this->queueDir); + + // Attacker constructs a Job with `command='system'` and signs it with + // their guessed key. With HMAC verification, the forged blob is + // rejected and we fall through to the structured-fields rebuild. + $forgedJob = new Job('system', ['rm -rf /'], 'pwn'); + $forgedSerialized = serialize($forgedJob); + $item = [ + 'serialized_job' => base64_encode($forgedSerialized), + 'serialized_job_hmac' => hash_hmac('sha256', $forgedSerialized, 'attacker-key-guess'), + 'job_id' => 'job-2', + // Legitimate fallback fields the queue would normally have. + 'command' => 'echo', + 'arguments' => ['safe'], + ]; + + $reconstructed = $this->reconstruct($queue, $item); + self::assertInstanceOf(Job::class, $reconstructed); + self::assertNotSame('system', $reconstructed->getCommand(), 'forged command must not survive'); + self::assertSame('echo', $reconstructed->getCommand(), 'must rebuild from structured fallback fields'); + } + + public function testReconstructJob_GHSAvj3m_RejectsItemMissingHmacField(): void + { + $queue = new JobQueue($this->queueDir); + + // Pre-fix queue files only carried `serialized_job`; with the fix in + // place those entries can no longer trigger unserialize, but if a + // structured fallback exists they still execute via that path. + $forgedJob = new Job('system', ['rm -rf /'], 'pwn'); + $item = [ + 'serialized_job' => base64_encode(serialize($forgedJob)), + 'job_id' => 'job-3', + 'command' => 'echo', + 'arguments' => ['safe'], + ]; + + $reconstructed = $this->reconstruct($queue, $item); + self::assertInstanceOf(Job::class, $reconstructed); + self::assertSame('echo', $reconstructed->getCommand()); + } + + public function testReconstructJob_GHSAvj3m_ReturnsNullOnFullyTamperedItem(): void + { + $queue = new JobQueue($this->queueDir); + + // No HMAC, no fallback fields — nothing to safely reconstruct from. + $item = [ + 'serialized_job' => base64_encode(serialize(new Job('system', ['x'], 'p'))), + 'job_id' => 'job-4', + ]; + + self::assertNull($this->reconstruct($queue, $item)); + } + + // ========================================================================= + // GHSA-vj3m-2g9h-vm4p (#3): Session::getFlashObject HMAC integrity + // ========================================================================= + // + // We can't easily exercise Session through its full PHP-session lifecycle + // in a unit test, so we verify the wire format directly: setFlashObject + // produces a `v2|<hmac>|<serialized>` envelope, and getFlashObject only + // accepts payloads whose HMAC verifies against Security::getNonceKey(). + + public function testSetFlashObject_GHSAvj3m_WrapsPayloadWithVersionedHmacEnvelope(): void + { + $session = $this->newSessionStub(); + $session->setFlashObject('payload', ['hello' => 'world']); + + $stored = $session->_storage['payload'] ?? null; + self::assertIsString($stored); + self::assertStringStartsWith('v2|', $stored); + + $parts = explode('|', $stored, 3); + self::assertCount(3, $parts); + [, $hmac, $serialized] = $parts; + self::assertSame( + hash_hmac('sha256', $serialized, Security::getNonceKey()), + $hmac, + 'envelope HMAC must match Security::getNonceKey()' + ); + self::assertSame(['hello' => 'world'], unserialize($serialized, ['allowed_classes' => false])); + } + + public function testGetFlashObject_GHSAvj3m_RoundTripsValidValue(): void + { + $session = $this->newSessionStub(); + $session->setFlashObject('payload', ['hello' => 'world']); + + self::assertSame(['hello' => 'world'], $session->getFlashObject('payload')); + } + + public function testGetFlashObject_GHSAvj3m_RejectsLegacyUnsignedPayload(): void + { + $session = $this->newSessionStub(); + // Pre-fix wire format: a bare serialize() blob with no envelope. + $session->_storage['payload'] = serialize(['hello' => 'world']); + + self::assertNull($session->getFlashObject('payload'), 'legacy unsigned payload must not be unserialized'); + } + + public function testGetFlashObject_GHSAvj3m_RejectsForgedHmac(): void + { + $session = $this->newSessionStub(); + $serialized = serialize(['attacker' => 'payload']); + $forgedHmac = hash_hmac('sha256', $serialized, 'wrong-key'); + $session->_storage['payload'] = "v2|{$forgedHmac}|{$serialized}"; + + self::assertNull($session->getFlashObject('payload'), 'forged HMAC must be rejected'); + } + + /** + * Minimal stub that mimics the bits of Grav\Common\Session that + * setFlashObject/getFlashObject touch (just `__get`/`__set`/`__unset` + * over an in-memory array). Avoids booting PHP session machinery. + */ + private function newSessionStub(): object + { + return new class extends \Grav\Common\Session { + public array $_storage = []; + + public function __construct() {} + + public function __set($name, $value): void + { + $this->_storage[$name] = $value; + } + + public function __get($name) + { + return $this->_storage[$name] ?? null; + } + + public function __unset($name): void + { + unset($this->_storage[$name]); + } + + public function __isset($name): bool + { + return isset($this->_storage[$name]); + } + }; + } +}
d904efc33e03Security fixes: FormFlash traversal, salt leak, user-overwrite hardening
13 files changed · +547 −33
CHANGELOG.md+4 −0 modified@@ -9,6 +9,10 @@ * Smarter dangerous-Twig filter — fixes false positives like `{{ page.header.user.mail }}` getting blocked because they happened to contain a dangerous function name. * Soft-fail on sandbox violations — the rest of the page still renders, visitors see a small placeholder, admins see a pointer to the log entry. * Escape hatch — the sandbox can be disabled from the admin UI (or YAML) if a site genuinely needs the old, unrestricted behaviour. +1. [](#bugfix) + * [security] Fixed unauthenticated path traversal in `FormFlash` (GHSA-hmcx-ch82-3fv2): `session_id` and `unique_id` now pass through a strict allowlist before being used to build on-disk paths, preventing arbitrary directory creation via the `__form-flash-id` parameter. + * [security] Fixed salt disclosure via sandboxed Twig (GHSA-3f29-pqwf-v4j4): the HMAC key used for CSRF nonces and admin rate-limit hashing has moved out of Config into `user/config/security-private.php` (blocked from web access by the default `user/*.php` htaccess rule), so `{{ grav.config.get('security.salt') }}` no longer leaks it. Existing sites are migrated automatically on first request — existing nonces and sessions survive the upgrade. + * [security] Hardened the new-user uniqueness guard in `UserObject::save` (GHSA-rr73-568v-28f8): `strpos(..., '@@')` replaced with `str_contains` (the old form was falsy when the transient-key marker was at position 0, silently bypassing the check) and the check now runs for every `FlexStorageInterface` backend rather than only `FileStorage`. A low-privileged user with `admin.users.create` can no longer disrupt a super-admin account by submitting the admin's username through the "add user" form. # v2.0.0-beta.1 ## 04/16/2026
.gitignore+2 −0 modified@@ -26,6 +26,8 @@ user/plugins/* user/themes/* !user/themes/.* user/**/config/security.yaml +user/**/config/security-private.php +user/**/config/security-private.php.tmp # Environments .env
system/config/security.yaml+4 −1 modified@@ -298,4 +298,7 @@ twig_sandbox: # Wildcard on stdClass for {{ page.header.<any_yaml_key> }} - class: 'stdClass' methods: '*' -salt: SbmgUJQ62MqNc0 +# NOTE: `salt:` was removed in v2.0.0-beta.2 (GHSA-3f29-pqwf-v4j4). The equivalent +# HMAC key is now stored in `user/config/security-private.php`, outside the Config +# tree, so sandboxed Twig cannot read it via `grav.config.get(...)`. On upgrade, +# any legacy `salt:` in `user/config/security.yaml` is migrated automatically.
system/src/Grav/Common/Config/Setup.php+3 −16 modified@@ -12,7 +12,6 @@ use BadMethodCallException; use Grav\Common\File\CompiledYamlFile; use Grav\Common\Data\Data; -use Grav\Common\Utils; use InvalidArgumentException; use Pimple\Container; use Psr\Http\Message\ServerRequestInterface; @@ -407,21 +406,9 @@ protected function check(UniformResourceLocator $locator) $this->initializeLocator($locator); } - // Create security.yaml salt if it doesn't exist into existing configuration environment if possible. - $securityFile = Utils::basename(static::$securityFile); - $securityFolder = substr(static::$securityFile, 0, -\strlen($securityFile)); - $securityFolder = $locator->findResource($securityFolder, true) ?: $locator->findResource($securityFolder, true, true); - $filename = "{$securityFolder}/{$securityFile}"; - - $security_file = CompiledYamlFile::instance($filename); - $security_content = (array)$security_file->content(); - - if (!isset($security_content['salt'])) { - $security_content = array_merge($security_content, ['salt' => Utils::generateRandomString(14)]); - $security_file->content($security_content); - $security_file->save(); - $security_file->free(); - } + // Legacy `security.salt` auto-gen was removed in v2.0 (GHSA-3f29-pqwf-v4j4); + // Security::getNonceKey() now manages the equivalent value in a private + // PHP file outside the Config tree so sandboxed Twig cannot read it. } catch (RuntimeException $e) { throw new RuntimeException(sprintf('Grav failed to initialize: %s', $e->getMessage()), 500, $e); }
system/src/Grav/Common/Flex/Types/Users/UserObject.php+14 −11 modified@@ -37,7 +37,6 @@ use Grav\Framework\Filesystem\Filesystem; use Grav\Framework\Flex\Flex; use Grav\Framework\Flex\FlexDirectory; -use Grav\Framework\Flex\Storage\FileStorage; use Grav\Framework\Flex\Traits\FlexMediaTrait; use Grav\Framework\Flex\Traits\FlexRelationshipsTrait; use Grav\Framework\Form\FormFlashFile; @@ -599,20 +598,24 @@ public function save() { // TODO: We may want to handle this in the storage layer in the future. $key = $this->getStorageKey(); - $isNewUser = !$key || strpos($key, '@@'); + // `@@` is Flex's marker for an in-memory (not-yet-persisted) storage key. + // `str_contains` is the correct predicate here — `strpos` returns 0 when + // the marker is at position 0 (e.g. `@@hash`), which is falsy and would + // have skipped the uniqueness check below. + $isNewUser = $key === '' || str_contains($key, '@@'); if ($isNewUser) { - $storage = $this->getFlexDirectory()->getStorage(); - if ($storage instanceof FileStorage) { - $newKey = $this->getKey(); - - // Check if a user with this username already exists (prevent overwriting) - if ($storage->hasKey($newKey)) { - throw new RuntimeException('User account with this username already exists'); - } + $newKey = $this->getKey(); - $this->setStorageKey($newKey); + // Prevent overwriting an existing account when a low-privileged user + // creates a new user with an already-taken username (GHSA-rr73-568v-28f8). + // Applies to every storage implementation, not just FileStorage. + $storage = $this->getFlexDirectory()->getStorage(); + if ($storage->hasKey($newKey)) { + throw new RuntimeException('User account with this username already exists'); } + + $this->setStorageKey($newKey); } $password = $this->getProperty('password') ?? $this->getProperty('password1');
system/src/Grav/Common/Security.php+94 −0 modified@@ -15,6 +15,9 @@ use Grav\Common\Page\Pages; use Grav\Common\Twig\Sandbox\GravSecurityPolicy; use Rhukster\DomSanitizer\DOMSanitizer; +use RocketTheme\Toolbox\File\YamlFile; +use RocketTheme\Toolbox\ResourceLocator\UniformResourceLocator; +use RuntimeException; use Twig\Sandbox\SecurityPolicyInterface; use function chr; use function count; @@ -808,4 +811,95 @@ private static function splitMethodNames($methods, bool $lowercase = true): arra } return $clean; } + + /** @var string|null in-process cache for the nonce key */ + private static ?string $nonceKey = null; + + /** + * Per-site HMAC key used for CSRF nonce signing, admin rate-limit key hashing, + * and (when configured) session-name derivation. Backed by a local PHP file + * outside the Config tree, so sandboxed Twig cannot reach it via + * `grav.config.get('security.salt')` or `Config::toArray()` (GHSA-3f29-pqwf-v4j4). + * + * Migration: if the legacy `security.salt` key is present in the loaded Config + * (i.e. from an older install's `user/config/security.yaml`), its value is + * copied into the private file on first call and scrubbed from both the live + * Config and the on-disk YAML. Existing CSRF nonces and sessions survive the + * upgrade because the key value is preserved. + * + * To rotate the key manually, delete `user/config/security-private.php`; the + * next request generates a fresh 64-char random value. Rotation invalidates + * in-flight CSRF nonces and — if `system.session.uniqueness` is set to + * `security` — existing sessions. + */ + public static function getNonceKey(): string + { + if (self::$nonceKey !== null) { + return self::$nonceKey; + } + + $grav = Grav::instance(); + /** @var UniformResourceLocator $locator */ + $locator = $grav['locator']; + $configFolder = $locator->findResource('config://', true) ?: $locator->findResource('config://', true, true); + $privateFile = "{$configFolder}/security-private.php"; + + if (is_file($privateFile)) { + $value = @include $privateFile; + if (is_string($value) && $value !== '') { + return self::$nonceKey = $value; + } + // Corrupt/empty file — fall through to regenerate. + } + + // One-time migration out of Config for sites upgrading from <= v2.0.0-beta.2. + /** @var Config $config */ + $config = $grav['config']; + $legacy = $config->get('security.salt'); + if (is_string($legacy) && $legacy !== '') { + self::writeNonceKey($privateFile, $legacy); + $config->set('security.salt', null); + + $securityYaml = "{$configFolder}/security.yaml"; + if (is_file($securityYaml)) { + $file = YamlFile::instance($securityYaml); + $content = (array) $file->content(); + if (array_key_exists('salt', $content)) { + unset($content['salt']); + $file->content($content); + $file->save(); + $file->free(); + } + } + + return self::$nonceKey = $legacy; + } + + $generated = bin2hex(random_bytes(32)); + self::writeNonceKey($privateFile, $generated); + + return self::$nonceKey = $generated; + } + + private static function writeNonceKey(string $path, string $value): void + { + $escaped = var_export($value, true); + $contents = "<?php\n\n// Auto-generated private secret. Do NOT commit to version control.\n// Used for CSRF nonce signing and admin rate-limit hashing. Regenerate by\n// deleting this file; the next request will write a new value.\n\nreturn {$escaped};\n"; + + $dir = dirname($path); + if (!is_dir($dir)) { + Folder::create($dir); + } + + // Atomic write: stage to a temp file, fsync via rename. + $tmp = $path . '.tmp'; + if (@file_put_contents($tmp, $contents, LOCK_EX) === false) { + throw new RuntimeException('Failed to write nonce key file'); + } + @chmod($tmp, 0600); + if (!@rename($tmp, $path)) { + @unlink($tmp); + throw new RuntimeException('Failed to commit nonce key file'); + } + } }
system/src/Grav/Common/Service/SessionServiceProvider.php+2 −1 modified@@ -11,6 +11,7 @@ use Grav\Common\Config\Config; use Grav\Common\Debugger; +use Grav\Common\Security; use Grav\Common\Session; use Grav\Common\Uri; use Grav\Common\Utils; @@ -86,7 +87,7 @@ public function register(Container $container) } $session_prefix = $c['inflector']->hyphenize($config->get('system.session.name', 'grav-site')); - $session_uniqueness = $config->get('system.session.uniqueness', 'path') === 'path' ? substr(md5(GRAV_ROOT), 0, 7) : md5((string) $config->get('security.salt')); + $session_uniqueness = $config->get('system.session.uniqueness', 'path') === 'path' ? substr(md5(GRAV_ROOT), 0, 7) : md5(Security::getNonceKey()); $session_name = $session_prefix . '-' . $session_uniqueness;
system/src/Grav/Common/Utils.php+1 −1 modified@@ -1405,7 +1405,7 @@ private static function generateNonceString($action, $previousTick = false) $i--; } - return ($i . '|' . $action . '|' . $username . '|' . $token . '|' . $grav['config']->get('security.salt')); + return ($i . '|' . $action . '|' . $username . '|' . $token . '|' . Security::getNonceKey()); } /**
system/src/Grav/Framework/Form/FormFlash.php+24 −3 modified@@ -75,9 +75,14 @@ public function __construct($config) $config = array_filter($config, static fn($val) => $val !== null); } - $this->id = $config['id'] ?? ''; - $this->sessionId = $config['session_id'] ?? ''; - $this->uniqueId = $config['unique_id'] ?? ''; + // Identifiers are used to build filesystem paths (tmp://forms/<sessionId>/<uniqueId>) + // and can reach this constructor from request input (e.g. __form-flash-id). + // Reject anything outside a strict alphanumeric+[,_-] allowlist to prevent path + // traversal into arbitrary directories. Invalid values blank out, which disables + // flash storage for the request rather than failing hard. + $this->id = self::sanitizeId($config['id'] ?? ''); + $this->sessionId = self::sanitizeId($config['session_id'] ?? ''); + $this->uniqueId = self::sanitizeId($config['unique_id'] ?? ''); $this->setUser($config['user'] ?? null); @@ -489,6 +494,22 @@ public function getTmpDir(): string return $this->folder && $this->uniqueId ? "{$this->folder}/{$this->uniqueId}" : ''; } + /** + * Gate for identifiers used in filesystem paths. Accepts the character + * set produced by PHP session IDs and Grav's form unique-id generators + * (alphanumerics, comma, underscore, hyphen). Anything else — including + * empty/non-string values — collapses to an empty string, which causes + * save()/delete()/getTmpDir() to become no-ops. + */ + private static function sanitizeId($id): string + { + if (!is_string($id) || $id === '') { + return ''; + } + + return preg_match('/^[A-Za-z0-9,_-]{1,64}$/', $id) ? $id : ''; + } + /** * @return ?YamlFile */
tests/unit/Grav/Common/Security/FormFlashSecurityTest.php+106 −0 added@@ -0,0 +1,106 @@ +<?php + +use Codeception\Util\Fixtures; +use Grav\Common\Grav; +use Grav\Framework\Form\FormFlash; + +/** + * Class FormFlashSecurityTest + * + * Covers: GHSA-hmcx-ch82-3fv2 (unauthenticated path traversal / arbitrary + * directory creation via FormFlash session_id / unique_id). + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class FormFlashSecurityTest extends \PHPUnit\Framework\TestCase +{ + /** @var Grav */ + protected $grav; + + protected function setUp(): void + { + parent::setUp(); + $grav = Fixtures::get('grav'); + $this->grav = $grav(); + } + + // ========================================================================= + // GHSA-hmcx-ch82-3fv2: Unauthenticated path traversal in FormFlash + // ========================================================================= + + /** + * @dataProvider providerGHSAhmcx_TraversalIds + */ + public function testConstruct_GHSAhmcx_RejectsTraversalSessionId(string $id, string $description): void + { + $flash = new FormFlash([ + 'session_id' => $id, + 'unique_id' => 'abcdef1234567890', + 'form_name' => 'test', + ]); + + $this->assertSame('', $flash->getSessionId(), $description); + $this->assertSame('', $flash->getTmpDir(), "tmp dir must be empty when session id is rejected: {$description}"); + } + + /** + * @dataProvider providerGHSAhmcx_TraversalIds + */ + public function testConstruct_GHSAhmcx_RejectsTraversalUniqueId(string $id, string $description): void + { + $flash = new FormFlash([ + 'session_id' => 'abcdef1234567890', + 'unique_id' => $id, + 'form_name' => 'test', + ]); + + $this->assertSame('', $flash->getUniqueId(), $description); + $this->assertSame('', $flash->getTmpDir(), "tmp dir must be empty when unique id is rejected: {$description}"); + } + + /** + * @dataProvider providerGHSAhmcx_ValidIds + */ + public function testConstruct_GHSAhmcx_AcceptsValidIds(string $id, string $description): void + { + $flash = new FormFlash([ + 'session_id' => $id, + 'unique_id' => $id, + 'form_name' => 'test', + ]); + + $this->assertSame($id, $flash->getSessionId(), $description); + $this->assertSame($id, $flash->getUniqueId(), $description); + } + + public static function providerGHSAhmcx_TraversalIds(): array + { + return [ + ['../../user/config/proof_dir', 'parent-dir traversal (advisory PoC)'], + ['..', 'bare parent-dir'], + ['foo/../bar', 'embedded traversal'], + ['foo/bar', 'embedded forward slash'], + ['foo\\bar', 'embedded backslash'], + ['foo:bar', 'colon (stream prefix)'], + ['foo.bar', 'embedded dot'], + ['tmp://forms/abc', 'explicit stream url'], + ['%2e%2e%2f', 'url-encoded traversal'], + [str_repeat('a', 65), 'overlong (>64 chars)'], + ["abc\x00def", 'null byte'], + ["abc\ndef", 'newline'], + [' abc', 'leading space'], + ]; + } + + public static function providerGHSAhmcx_ValidIds(): array + { + return [ + ['abc123', 'alphanumeric'], + ['ABCdef123', 'mixed case'], + ['session-id-with-dashes', 'hyphens'], + ['session_id_with_underscores', 'underscores'], + ['session,id,with,commas', 'commas (PHP 5-bit session id)'], + [str_repeat('a', 64), 'max length (64 chars)'], + ]; + } +}
tests/unit/Grav/Common/Security/NonceKeySecurityTest.php+136 −0 added@@ -0,0 +1,136 @@ +<?php + +use Codeception\Util\Fixtures; +use Grav\Common\Grav; +use Grav\Common\Security; +use Grav\Common\Filesystem\Folder; + +/** + * Class NonceKeySecurityTest + * + * Covers: GHSA-3f29-pqwf-v4j4 (salt leak via sandboxed Twig `grav.config.get('security.salt')`). + * + * Verifies `Security::getNonceKey()`: + * - reads the value from `user/config/security-private.php` when present, + * - migrates a legacy `security.salt` out of Config on first call (preserving the value + * so existing nonces/sessions survive), and scrubs it from the loaded Config, + * - generates a fresh 64-char hex value for a clean install, + * - is stable within a request (subsequent calls return the same value). + */ +class NonceKeySecurityTest extends \PHPUnit\Framework\TestCase +{ + /** @var Grav */ + protected $grav; + + /** @var string */ + protected $configFolder; + + /** @var string */ + protected $privateFile; + + protected function setUp(): void + { + parent::setUp(); + $grav = Fixtures::get('grav'); + $this->grav = $grav(); + + /** @var \RocketTheme\Toolbox\ResourceLocator\UniformResourceLocator $locator */ + $locator = $this->grav['locator']; + $this->configFolder = $locator->findResource('config://', true) ?: $locator->findResource('config://', true, true); + $this->privateFile = "{$this->configFolder}/security-private.php"; + + // Ensure a clean slate. + $this->resetStaticCache(); + if (is_file($this->privateFile)) { + @unlink($this->privateFile); + } + if (is_file("{$this->privateFile}.tmp")) { + @unlink("{$this->privateFile}.tmp"); + } + $this->grav['config']->set('security.salt', null); + } + + protected function tearDown(): void + { + if (is_file($this->privateFile)) { + @unlink($this->privateFile); + } + if (is_file("{$this->privateFile}.tmp")) { + @unlink("{$this->privateFile}.tmp"); + } + // The migration test path can rewrite security.yaml to drop the salt key; + // delete the empty file so we don't leave per-environment residue behind. + $securityYaml = "{$this->configFolder}/security.yaml"; + if (is_file($securityYaml) && trim((string) @file_get_contents($securityYaml)) === '{ }') { + @unlink($securityYaml); + } + $this->resetStaticCache(); + parent::tearDown(); + } + + private function resetStaticCache(): void + { + $reflection = new ReflectionClass(Security::class); + if ($reflection->hasProperty('nonceKey')) { + $p = $reflection->getProperty('nonceKey'); + $p->setAccessible(true); + $p->setValue(null, null); + } + } + + // ========================================================================= + // GHSA-3f29-pqwf-v4j4: legacy security.salt migrated out of Config + // ========================================================================= + + public function testGetNonceKey_GHSA3f29_MigratesLegacySaltAndScrubsFromConfig(): void + { + $legacy = 'legacy-salt-value-from-existing-install'; + $this->grav['config']->set('security.salt', $legacy); + + $key = Security::getNonceKey(); + + self::assertSame($legacy, $key, 'value should be preserved so existing nonces/sessions survive'); + self::assertNull( + $this->grav['config']->get('security.salt'), + 'GHSA-3f29: security.salt must be scrubbed from live Config after migration (sandboxed Twig cannot read it)' + ); + self::assertFileExists($this->privateFile, 'migration writes the value to the private file'); + } + + public function testGetNonceKey_GHSA3f29_ReadsFromPrivateFileOnSubsequentCalls(): void + { + $value = str_repeat('a', 64); + file_put_contents($this->privateFile, "<?php\nreturn " . var_export($value, true) . ";\n"); + + self::assertSame($value, Security::getNonceKey()); + } + + public function testGetNonceKey_GeneratesFreshKeyForCleanInstall(): void + { + // No legacy salt, no private file. + $key = Security::getNonceKey(); + + self::assertMatchesRegularExpression('/^[0-9a-f]{64}$/', $key, 'fresh install should generate 64-char hex key'); + self::assertFileExists($this->privateFile); + + // Re-read via a fresh cache to confirm persistence. + $this->resetStaticCache(); + self::assertSame($key, Security::getNonceKey(), 'value must persist across cache resets (i.e. across requests)'); + } + + public function testGetNonceKey_IsStableWithinRequest(): void + { + $a = Security::getNonceKey(); + $b = Security::getNonceKey(); + self::assertSame($a, $b); + } + + public function testGetNonceKey_IgnoresEmptyStringInConfig(): void + { + // An empty `salt:` in user config should NOT be migrated; we should generate instead. + $this->grav['config']->set('security.salt', ''); + + $key = Security::getNonceKey(); + self::assertMatchesRegularExpression('/^[0-9a-f]{64}$/', $key); + } +}
tests/unit/Grav/Common/Security/TwigSandboxTest.php+23 −0 modified@@ -174,6 +174,29 @@ public function testBuildPolicy_AllowsConfigGet(): void $this->assertDoesNotThrow(fn() => $policy->checkMethodAllowed($config, 'get')); } + /** + * GHSA-58hj-46fw-rcfm: a low-privilege editor with page-update access + * injected `{% set x = grav['accounts'].load(...) %} {{ x.set(...) }} {{ x.save() }}` + * from page content to mint a super-admin. The sandbox allows + * `grav.offsetGet('accounts')` (container traversal is deliberately permitted), + * but the returned UserCollection is not in `allowed_methods`, so `load()` / + * `save()` / `set()` on it must be denied. + */ + public function testBuildPolicy_GHSA58hj_BlocksAccountsCollectionMethods(): void + { + $policy = Security::buildTwigSandboxPolicy(); + $accounts = new \Grav\Common\User\DataUser\UserCollection(\Grav\Common\User\User::class); + + foreach (['load', 'save', 'set', '__set'] as $method) { + try { + $policy->checkMethodAllowed($accounts, $method); + self::fail("UserCollection::{$method} must be blocked (GHSA-58hj-46fw-rcfm)"); + } catch (SecurityNotAllowedMethodError $e) { + $this->addToAssertionCount(1); + } + } + } + // ========================================================================= // End-to-end render: SecurityError is raised when a disallowed primitive runs // =========================================================================
tests/unit/Grav/Common/Security/UserOverwriteSecurityTest.php+134 −0 added@@ -0,0 +1,134 @@ +<?php + +use Grav\Common\Flex\Types\Users\UserObject; +use Grav\Framework\Flex\FlexDirectory; +use Grav\Framework\Flex\Interfaces\FlexStorageInterface; + +/** + * Test double that lets us drive UserObject::save() without a full Flex + * directory/blueprint/validator stack. We override the handful of accessors + * the uniqueness-guard block reads. + */ +class StubUserObject extends UserObject +{ + public static ?FlexStorageInterface $stubStorage = null; + public static ?FlexDirectory $stubDirectory = null; + public string $stubKey = ''; + public string $stubStorageKey = ''; + + // Match FlexObjectInterface constructor signature, but skip all parent work. + public function __construct(array $elements = [], $key = '', ?FlexDirectory $directory = null, bool $validate = false) + { + // Deliberately empty — we don't want blueprint/validator/container boot. + } + + public function getKey(): string + { + return $this->stubKey; + } + + public function getStorageKey(): string + { + return $this->stubStorageKey; + } + + public function setStorageKey($key = null) + { + $this->stubStorageKey = (string)($key ?? ''); + return $this; + } + + public function getFlexDirectory(?string $type = null): FlexDirectory + { + return self::$stubDirectory; + } +} + +/** + * Class UserOverwriteSecurityTest + * + * Covers: GHSA-rr73-568v-28f8 (privilege de-escalation / admin account disruption + * by creating a user with an existing username and overwriting the target). + * + * Verifies the guard in UserObject::save() that refuses to create a new user + * whose chosen username is already taken. The test also pins the two subtle + * properties of the check: + * - `@@`-prefixed transient storage keys (Flex's in-memory marker for + * unsaved objects) are treated as "new user" and trigger the check — + * `strpos($key, '@@')` would return 0 here and be falsy, bypassing the check. + * - The uniqueness check runs for any FlexStorageInterface, not just FileStorage. + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class UserOverwriteSecurityTest extends \PHPUnit\Framework\TestCase +{ + private function makeStubbedUser(string $storageKey, string $targetKey, bool $targetExists): StubUserObject + { + $storage = $this->createMock(FlexStorageInterface::class); + $storage->method('hasKey')->willReturnCallback( + static fn(string $k): bool => $k === $targetKey && $targetExists + ); + + $directory = $this->createMock(FlexDirectory::class); + $directory->method('getStorage')->willReturn($storage); + + StubUserObject::$stubStorage = $storage; + StubUserObject::$stubDirectory = $directory; + + $user = new StubUserObject(); + $user->stubStorageKey = $storageKey; + $user->stubKey = $targetKey; + + return $user; + } + + // ========================================================================= + // GHSA-rr73-568v-28f8: create-new path must refuse a taken username + // ========================================================================= + + public function testSave_GHSArr73_BlocksNewUserWithExistingUsername(): void + { + $user = $this->makeStubbedUser(storageKey: '', targetKey: 'root0', targetExists: true); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('User account with this username already exists'); + $user->save(); + } + + public function testSave_GHSArr73_BlocksWhenTransientKeyStartsWithAtMarker(): void + { + // Flex's in-memory marker for unsaved objects is `@@<hash>`. `strpos($key, '@@')` + // returns 0 here, which is falsy — the old check would have skipped the guard + // and let the overwrite through. Pin the str_contains() fix. + $user = $this->makeStubbedUser(storageKey: '@@abc123', targetKey: 'root0', targetExists: true); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('User account with this username already exists'); + $user->save(); + } + + public function testSave_GHSArr73_AllowsNewUserWhenUsernameIsFree(): void + { + // When the username is NOT taken, the uniqueness check should pass and + // save() must be allowed to proceed past the guard. We can't run the + // rest of save() without a full Flex setup, so we assert that the + // specific RuntimeException is NOT thrown before control leaves our + // instrumented code — any other exception downstream is acceptable. + $user = $this->makeStubbedUser(storageKey: '', targetKey: 'brand-new-user', targetExists: false); + + try { + $user->save(); + $this->addToAssertionCount(1); + } catch (RuntimeException $e) { + $this->assertStringNotContainsString( + 'User account with this username already exists', + $e->getMessage(), + 'Uniqueness guard must not fire when the target username is free' + ); + } catch (\Throwable) { + // Downstream failures (missing blueprint, storage, events) are expected + // and do not indicate a guard regression. + $this->addToAssertionCount(1); + } + } +}
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
5- github.com/getgrav/grav/security/advisories/GHSA-hmcx-ch82-3fv2nvdExploitVendor AdvisoryWEB
- github.com/advisories/GHSA-hmcx-ch82-3fv2ghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2026-42608ghsaADVISORY
- github.com/getgrav/grav/commit/c66dfeb5ff679a1667678c6335eb9ff3255dfc47ghsaWEB
- github.com/getgrav/grav/commit/d904efc33e03ebb597afde8d3368b28cf0423632ghsaWEB
News mentions
0No linked articles in our index yet.