CVE-2026-7317
Description
A vulnerability was found in Grav CMS up to 1.7.49.5/2.0.0-beta.1. Affected by this vulnerability is the function FileCache::doGet of the file system/src/Grav/Framework/Cache/Adapter/FileCache.php of the component Cache Value Handler. The manipulation results in deserialization. The attack may be launched remotely. The attack requires a high level of complexity. The exploitation appears to be difficult. The exploit has been made public and could be used. Upgrading to version 2.0.0-beta.2 addresses this issue. The patch is identified as c66dfeb5f. The affected component should be upgraded.
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
25a12f9be8314Security fixes: XSS detection, SVG XXE, Zip Slip, media attribute
10 files changed · +584 −6
CHANGELOG.md+5 −0 modified@@ -15,6 +15,11 @@ * [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. + * [security] Tightened `Security::detectXss` `on_events` regex (GHSA-9695-8fr9-hw5q): the previous form required quotes/whitespace around the `=` sign and was bypassed by `<img onerror=alert(1)>`. Same fix knocks down the regex-bypass half of GHSA-c2q3-p4jr-c55f and GHSA-w8cg-7jcj-4vv2. + * [security] Added `svg`, `math`, `option`, `select` to default `security.xss_dangerous_tags` (GHSA-w8cg-7jcj-4vv2, GHSA-c2q3-p4jr-c55f) — XML-namespace tags that allow inline scripting plus the select-context break used to escape admin form templates. + * [security] `MediaObjectTrait::attribute()` now gates the attribute name through a strict identifier regex + denylist (`on*`, `style`, `xmlns`, `srcdoc`, `formaction`) so a Markdown image like `)` no longer injects an event handler onto the rendered tag (GHSA-r7fx-8g49-7hhr). + * [security] Hardened SVG dimension reading in `VectorImageMedium` against XXE / billion-laughs (GHSA-3446-6mgw-f79p): DOCTYPE/ENTITY declarations are stripped before parse and `simplexml_load_string` is now called with `LIBXML_NONET` (and `libxml_disable_entity_loader` for PHP < 8). The companion fix lives in rhukster/dom-sanitizer for SVG sanitization paths. + * [security] `Installer::unZip` now pre-validates every archive entry and refuses Zip Slip primitives — `..` segments, absolute paths, Windows drive letters, NUL bytes (GHSA-w48r-jppp-rcfw). Note: this hardens the path layer only; a well-formed but malicious plugin whose own PHP code is the payload remains a "trust the source" problem when using directInstall. # v2.0.0-beta.1 ## 04/16/2026
system/config/security.yaml+10 −0 modified@@ -32,6 +32,16 @@ xss_dangerous_tags: - title - base - isindex + # GHSA-w8cg-7jcj-4vv2: svg/math allow inline scripting via XML namespaces + # and event-handler attributes; flag them as dangerous in user-editable + # content so the on_events regex bypass (now fixed for GHSA-9695) has + # belt-and-suspenders coverage. + - svg + - math + # GHSA-c2q3-p4jr-c55f: option/select are used by attackers to break out + # of the admin select-template context. + - option + - select uploads_dangerous_extensions: - php - php2
system/src/Grav/Common/GPM/Installer.php+47 −0 modified@@ -179,6 +179,24 @@ public static function unZip($zip_file, $destination) $archive = $zip->open($zip_file); if ($archive === true) { + // GHSA-w48r-jppp-rcfw: validate every entry name before extraction. + // ZipArchive::extractTo would otherwise honour `../` segments and + // absolute paths, letting a crafted plugin/theme ZIP write files + // anywhere the web server can reach (Zip Slip, CVE-2018-1000544 + // family). Note: this hardens the path layer; it does NOT and + // cannot defend against a well-formed but malicious plugin whose + // own PHP code is the payload — that's a "trust the source" + // problem the admin must own when using directInstall. + $numFiles = $zip->numFiles; + for ($i = 0; $i < $numFiles; $i++) { + $entryName = (string) $zip->getNameIndex($i); + if (!self::isSafeArchiveEntry($entryName)) { + self::$error = self::ZIP_EXTRACT_ERROR; + $zip->close(); + return false; + } + } + Folder::create($destination); $unzip = $zip->extractTo($destination); @@ -207,6 +225,35 @@ public static function unZip($zip_file, $destination) return false; } + /** + * Reject Zip Slip primitives in archive entry names: empty names, NUL + * bytes, absolute paths, or any path segment that is `..`. Forward and + * back slashes are both treated as separators so Windows-authored + * archives are also covered. + * + * @internal Public for testing. + */ + public static function isSafeArchiveEntry(string $name): bool + { + if ($name === '' || str_contains($name, "\0")) { + return false; + } + if (str_starts_with($name, '/') || str_starts_with($name, '\\')) { + return false; + } + // Windows drive letter: C:\..., D:/... + if (preg_match('#^[A-Za-z]:[/\\\\]#', $name) === 1) { + return false; + } + // Any `..` path segment, regardless of slash flavour. + foreach (preg_split('#[\\\\/]+#', $name) as $segment) { + if ($segment === '..') { + return false; + } + } + return true; + } + /** * Instantiates and returns the package installer class *
system/src/Grav/Common/Media/Traits/MediaObjectTrait.php+35 −2 modified@@ -325,18 +325,51 @@ public function reset() /** * Add custom attribute to medium. * + * Reachable from Markdown via `?attribute=name,value` on image excerpts, so + * the attribute NAME is editor-controlled. We restrict it to plain HTML + * attribute identifiers (alphanumerics + `-`/`:`/`_`) and reject any name + * that would inject script when rendered onto an `<img>` tag — event + * handlers (`on*`), inline style, the XML namespace, srcdoc, and the + * various `form*` attributes whose URL targets are themselves trusted as + * actions. GHSA-r7fx-8g49-7hhr. + * * @param string $attribute * @param string $value * @return $this */ public function attribute($attribute = null, $value = '') { - if (!empty($attribute)) { - $this->attributes[$attribute] = $value; + if (empty($attribute) || !is_string($attribute)) { + return $this; } + if (!self::isSafeAttributeName($attribute)) { + return $this; + } + $this->attributes[$attribute] = $value; return $this; } + /** @internal */ + private static function isSafeAttributeName(string $name): bool + { + // Strict shape: HTML attribute names are letter-led, then alnum/-/_/:/./$ + // Anything else (whitespace, quotes, `<>`, etc.) is rejected outright. + if (!preg_match('/^[A-Za-z][A-Za-z0-9_:.\-]*$/', $name)) { + return false; + } + $lower = strtolower($name); + // Event handlers — primary GHSA-r7fx-8g49-7hhr vector. + if (str_starts_with($lower, 'on')) { + return false; + } + // Attribute names that open a scripting context regardless of value. + // We deliberately do NOT deny `src` / `href` here — themes legitimately + // call `$image->attribute('src', $signed_url)` from PHP, and the + // primary script-injection surface is the event-handler family above. + $denylist = ['style', 'xmlns', 'srcdoc', 'formaction']; + return !in_array($lower, $denylist, true); + } + /** * Switch display mode. *
system/src/Grav/Common/Page/Medium/VectorImageMedium.php+18 −1 modified@@ -46,7 +46,24 @@ public function __construct($items = [], ?Blueprint $blueprint = null) return; } - $xml = simplexml_load_string(file_get_contents($path)); + // GHSA-3446-6mgw-f79p: strip DOCTYPE/ENTITY declarations and pass + // LIBXML_NONET to prevent XXE / billion-laughs / network exfiltration + // when reading width/height from an attacker-supplied SVG. + $svg = (string) file_get_contents($path); + $svg = preg_replace('/<!DOCTYPE\b[^>]*(?:\[[^\]]*\])?[^>]*>/is', '', $svg) ?? $svg; + $svg = preg_replace('/<!ENTITY\b[^>]*>/i', '', $svg) ?? $svg; + + $previousEntityLoader = null; + if (\PHP_VERSION_ID < 80000 && function_exists('libxml_disable_entity_loader')) { + $previousEntityLoader = libxml_disable_entity_loader(true); + } + try { + $xml = simplexml_load_string($svg, 'SimpleXMLElement', LIBXML_NONET | LIBXML_NOERROR | LIBXML_NOWARNING); + } finally { + if ($previousEntityLoader !== null) { + libxml_disable_entity_loader($previousEntityLoader); + } + } $attr = $xml ? $xml->attributes() : null; if (!$attr instanceof \SimpleXMLElement) { return;
system/src/Grav/Common/Security.php+10 −3 modified@@ -229,9 +229,16 @@ public static function detectXss($string, ?array $options = null): ?string // Set the patterns we'll test against $patterns = [ - // Match any attribute starting with "on" or xmlns (must be preceded by whitespace/special chars) - // Allow optional whitespace between 'on' and event name to catch obfuscation attempts - 'on_events' => '#(<[^>]+[\s\x00-\x20\"\'\/])(on\s*[a-z]+|xmlns)\s*=[\s|\'\"].*[\s|\'\"]>#iUu', + // Match any attribute starting with "on" or xmlns (must be preceded by an + // attribute boundary: whitespace, NUL, quote or slash). We deliberately + // do NOT try to match the attribute value itself — the previous regex + // required quotes-or-spaces around the `=` sign and was bypassed by + // unquoted handlers like `<img src=x onerror=alert(1)>` + // (GHSA-9695-8fr9-hw5q, also exploited by GHSA-c2q3-p4jr-c55f and + // GHSA-w8cg-7jcj-4vv2). Detecting the attribute name + `=` is enough + // for a tripwire; trade-off is occasional false positives when an + // unrelated `on*=` substring appears inside another attribute's value. + 'on_events' => '#<[^>]*?[\s\x00-\x20\"\'\/](on\s*[a-z]+|xmlns)\s*=#iu', // Match javascript:, livescript:, vbscript:, mocha:, feed: and data: protocols 'invalid_protocols' => '#(' . implode('|', array_map('preg_quote', $invalid_protocols, ['#'])) . ')(:|\&\#58)\S.*?#iUu',
tests/unit/Grav/Common/Security/DetectXssTest.php+137 −0 added@@ -0,0 +1,137 @@ +<?php + +use Codeception\Util\Fixtures; +use Grav\Common\Grav; +use Grav\Common\Security; + +/** + * Class DetectXssTest + * + * Tests for Security::detectXss() — specifically the on_events regex hardening + * for GHSA-9695-8fr9-hw5q (unquoted event handlers), with parallel coverage + * for the same bypass pattern called out in GHSA-c2q3-p4jr-c55f and + * GHSA-w8cg-7jcj-4vv2. + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class DetectXssTest extends \PHPUnit\Framework\TestCase +{ + /** @var Grav */ + protected $grav; + + protected function setUp(): void + { + parent::setUp(); + $grav = Fixtures::get('grav'); + $this->grav = $grav(); + } + + // ========================================================================= + // GHSA-9695-8fr9-hw5q: unquoted on* handlers must be detected + // ========================================================================= + + /** + * @dataProvider providerGHSA9695_UnquotedOnEvents + */ + public function testDetectXss_GHSA9695_FlagsUnquotedEventHandler(string $payload, string $description): void + { + $result = Security::detectXss($payload); + self::assertSame('on_events', $result, "Should flag on_events for: $description"); + } + + public static function providerGHSA9695_UnquotedOnEvents(): array + { + return [ + ['<img src=x onerror=alert(1)>', 'advisory PoC: unquoted onerror, no space before >'], + ['<img src=x onerror=eval(atob(/Y/.source))>', 'advisory PoC: atob/regex.source obfuscation'], + ['<svg onload=alert(1)>', 'unquoted onload on svg'], + ['<body onload=alert(1)>', 'unquoted onload on body'], + ['<a href=# onclick=alert(1)>x</a>', 'unquoted onclick'], + // GHSA-c2q3-p4jr-c55f payload — the exact taxonomy escape sequence: + ['</option></select><img src=x onerror=alert(1)>', 'GHSA-c2q3 select-context break + unquoted onerror'], + // Obfuscation: whitespace inside the event name (e.g. on error=) + ['<img src=x on error=alert(1)>', 'whitespace between on and event name'], + // xmlns is also covered by the same rule — keep regression coverage: + ['<svg xmlns=http://example.com/ns>', 'unquoted xmlns'], + ]; + } + + /** + * @dataProvider providerGHSA9695_QuotedOnEvents + */ + public function testDetectXss_GHSA9695_StillFlagsQuotedEventHandlersAfterFix(string $payload, string $description): void + { + // Make sure tightening the regex didn't regress the previously-working + // quoted forms. + $result = Security::detectXss($payload); + self::assertSame('on_events', $result, "Should still flag quoted on_events for: $description"); + } + + public static function providerGHSA9695_QuotedOnEvents(): array + { + return [ + ['<img src="x" onerror="alert(1)">', 'double-quoted onerror'], + ["<img src='x' onerror='alert(1)'>", 'single-quoted onerror'], + ['<body onload="document.location=\'evil\'">', 'quoted onload'], + ['<svg onload="fetch(\'/x\')">', 'svg with quoted onload'], + ]; + } + + // ========================================================================= + // Negative coverage: legitimate content should not trip on_events + // ========================================================================= + + /** + * @dataProvider providerSafeContent + */ + public function testDetectXss_SafeContentReturnsNullOnEventsRule(string $payload, string $description): void + { + // Some safe content may still trip OTHER rules (e.g. the dangerous_tags + // list), but the on_events rule specifically should not fire. + $result = Security::detectXss($payload); + self::assertNotSame('on_events', $result, "on_events must not fire for: $description"); + } + + public static function providerSafeContent(): array + { + return [ + ['<p>Hello world</p>', 'plain paragraph'], + ['<a href="https://example.com">link</a>', 'link with href'], + ['<img src="/foo.png" alt="bar">', 'plain img'], + ['Pricing on demand', 'word starting with "on" outside any tag'], + ['<button>Click me</button>', 'button tag (ends in "on")'], + ['<section>content</section>', 'section tag'], + ]; + } + + // ========================================================================= + // GHSA-w8cg-7jcj-4vv2: svg/math + GHSA-c2q3 option/select added to defaults + // ========================================================================= + + /** + * @dataProvider providerGHSAw8cg_NewlyDangerousTags + */ + public function testDetectXss_GHSAw8cg_FlagsNewlyDangerousTags(string $payload, string $description): void + { + $result = Security::detectXss($payload); + // Either dangerous_tags (new) or on_events (already covered by #1) is + // an acceptable trip — both indicate the payload is flagged. + self::assertNotNull($result, "Should flag: $description"); + self::assertContains( + $result, + ['dangerous_tags', 'on_events'], + "Expected dangerous_tags or on_events for: $description, got '$result'" + ); + } + + public static function providerGHSAw8cg_NewlyDangerousTags(): array + { + return [ + ['<svg><script>alert(1)</script></svg>', 'GHSA-w8cg svg with embedded script'], + ['<svg></svg>', 'svg tag alone'], + ['<math><mtext>x</mtext></math>', 'math tag (similar XML namespace risk)'], + ['</option></select>injected', 'GHSA-c2q3 option/select context break'], + ['<select><option>x</option></select>', 'option/select wrapping'], + ]; + } +}
tests/unit/Grav/Common/Security/MediaAttributeSecurityTest.php+125 −0 added@@ -0,0 +1,125 @@ +<?php + +use Codeception\Util\Fixtures; +use Grav\Common\Grav; +use Grav\Common\Media\Traits\MediaObjectTrait; + +/** + * Class MediaAttributeSecurityTest + * + * Covers: GHSA-r7fx-8g49-7hhr (Markdown image `?attribute=NAME,VALUE` reaches + * `MediaObjectTrait::attribute()` which let an editor set arbitrary HTML + * attribute names — including event handlers — on the rendered <img>). + * + * The fix gates the attribute name through an allowlist regex and a small + * denylist of script-context names. Safe `data-*`/`aria-*`/typical media + * attributes still pass. + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class MediaAttributeSecurityTest extends \PHPUnit\Framework\TestCase +{ + /** @var Grav */ + protected $grav; + + protected function setUp(): void + { + parent::setUp(); + $grav = Fixtures::get('grav'); + $this->grav = $grav(); + } + + private function newMedium(): object + { + // Lightweight stand-in: any class that uses MediaObjectTrait works. + // attribute() reads/writes the trait's own protected $attributes; we + // expose it through reflection in the assertions. The trait declares + // a handful of abstract methods we don't exercise — stub them. + return new class { + use MediaObjectTrait; + public function addMetaFile($filepath) {} + public function __toString(): string { return ''; } + public function url($reset = true) { return ''; } + public function get($name, mixed $default = null, $separator = null) { return $default; } + public function set($name, mixed $value, $separator = null) { return $this; } + protected function createThumbnail($thumb) { return null; } + protected function createLink(array $attributes) { return null; } + protected function getItems(): array { return []; } + }; + } + + private function attrs(object $medium): array + { + $r = new ReflectionClass($medium); + $p = $r->getProperty('attributes'); + $p->setAccessible(true); + return (array) $p->getValue($medium); + } + + /** + * @dataProvider providerGHSAr7fx_DangerousAttributeNames + */ + public function testAttribute_GHSAr7fx_RejectsDangerousAttributeNames(string $name, string $description): void + { + $m = $this->newMedium(); + $m->attribute($name, 'value-that-must-not-stick'); + + self::assertArrayNotHasKey($name, $this->attrs($m), "Should not store dangerous attribute: $description"); + self::assertArrayNotHasKey(strtolower($name), $this->attrs($m), "case-insensitive: $description"); + } + + public static function providerGHSAr7fx_DangerousAttributeNames(): array + { + return [ + ['onerror', 'GHSA-r7fx PoC: onerror handler'], + ['onload', 'onload handler'], + ['onclick', 'onclick handler'], + ['ONERROR', 'uppercase event handler'], + ['OnMouseOver', 'mixed-case event handler'], + ['style', 'inline style (CSS expression risk)'], + ['xmlns', 'XML namespace'], + ['srcdoc', 'iframe srcdoc'], + ['formaction', 'form action override'], + // Malformed names — must be rejected even before the denylist hits. + ['bad name', 'whitespace in attribute name'], + ['bad>tag', 'attribute name with `>` (tag-break attempt)'], + ['"onerror', 'attribute name with leading quote'], + ['', 'empty name (already handled by empty() check)'], + ['1foo', 'attribute name not letter-led'], + ]; + } + + /** + * @dataProvider providerGHSAr7fx_SafeAttributeNames + */ + public function testAttribute_GHSAr7fx_AcceptsSafeAttributeNames(string $name, string $description): void + { + $m = $this->newMedium(); + $m->attribute($name, 'safe-value'); + + self::assertArrayHasKey($name, $this->attrs($m), "Should accept safe attribute: $description"); + self::assertSame('safe-value', $this->attrs($m)[$name], "value should round-trip: $description"); + } + + public static function providerGHSAr7fx_SafeAttributeNames(): array + { + return [ + ['alt', 'alt'], + ['title', 'title'], + ['class', 'class'], + ['id', 'id'], + ['loading', 'loading'], + ['decoding', 'decoding'], + ['width', 'width'], + ['height', 'height'], + ['data-foo', 'data-* attribute (common theme use)'], + ['data-image-id', 'data-* with hyphens'], + ['aria-label', 'aria-* attribute'], + ['rel', 'rel'], + // src/href intentionally allowed — themes legitimately call + // $image->attribute('src', $signed_url) from PHP. + ['src', 'src (themes override URLs)'], + ['href', 'href (link wrappers)'], + ]; + } +}
tests/unit/Grav/Common/Security/SvgXxeSecurityTest.php+126 −0 added@@ -0,0 +1,126 @@ +<?php + +/** + * Class SvgXxeSecurityTest + * + * Covers: GHSA-3446-6mgw-f79p (XXE via SVG upload). Grav reads dimensions + * from uploaded SVGs via simplexml_load_string in VectorImageMedium; the + * hardening pre-strips DOCTYPE/ENTITY declarations and parses with + * LIBXML_NONET so attacker-controlled `SYSTEM` entity references can't + * pull in `/etc/passwd` or trigger network requests. + * + * The hardening is applied in two places: VectorImageMedium itself and + * (independently) the dom-sanitizer library. This test file exercises the + * parsing primitives that both rely on; a separate test in dom-sanitizer's + * own suite covers that side. + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class SvgXxeSecurityTest extends \PHPUnit\Framework\TestCase +{ + /** + * Mirror VectorImageMedium's hardened SVG parse so the test exercises + * the exact strip-and-parse sequence we ship. + */ + private static function safeParseSvg(string $content): ?\SimpleXMLElement + { + $content = preg_replace('/<!DOCTYPE\b[^>]*(?:\[[^\]]*\])?[^>]*>/is', '', $content) ?? $content; + $content = preg_replace('/<!ENTITY\b[^>]*>/i', '', $content) ?? $content; + + $previousEntityLoader = null; + if (\PHP_VERSION_ID < 80000 && function_exists('libxml_disable_entity_loader')) { + $previousEntityLoader = libxml_disable_entity_loader(true); + } + try { + $xml = simplexml_load_string($content, 'SimpleXMLElement', LIBXML_NONET | LIBXML_NOERROR | LIBXML_NOWARNING); + } finally { + if ($previousEntityLoader !== null) { + libxml_disable_entity_loader($previousEntityLoader); + } + } + return $xml === false ? null : $xml; + } + + /** + * After DOCTYPE/ENTITY stripping, any `&name;` reference in the body + * becomes an undefined entity. simplexml may then refuse to parse the + * doc entirely (returning null) — which is itself a safe outcome: + * nothing was expanded, nothing leaked. We accept either result and + * focus the assertions on what MUST NOT happen (file contents leaking + * into the output). + */ + public function testParse_GHSA3446_DoesNotExpandExternalSystemEntity(): void + { + $payload = <<<'SVG' +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE svg [ + <!ENTITY xxe SYSTEM "file:///etc/passwd"> +]> +<svg xmlns="http://www.w3.org/2000/svg" width="&xxe;" height="100"> + <text>&xxe;</text> +</svg> +SVG; + $xml = self::safeParseSvg($payload); + + if ($xml === null) { + // Parser refused the doc (undefined &xxe; after strip) — safe. + $this->addToAssertionCount(1); + return; + } + + // If a parser was lenient enough to accept it, the entity must NOT + // have been substituted with /etc/passwd contents. + $width = (string) $xml->attributes()->width; + $textContent = (string) $xml->text; + self::assertStringNotContainsString('root:x:', $width, 'must not have read /etc/passwd into the width attribute'); + self::assertStringNotContainsString('root:x:', $textContent, 'must not have read /etc/passwd into <text>'); + self::assertStringNotContainsString('/bin/', $width); + self::assertStringNotContainsString('/bin/', $textContent); + } + + public function testParse_GHSA3446_BillionLaughsDoesNotExpand(): void + { + $payload = <<<'SVG' +<?xml version="1.0"?> +<!DOCTYPE lolz [ + <!ENTITY lol "lol"> + <!ENTITY lol2 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;"> + <!ENTITY lol3 "&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;"> +]> +<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"> + <text>&lol3;</text> +</svg> +SVG; + $startPeak = memory_get_peak_usage(); + $xml = self::safeParseSvg($payload); + $delta = memory_get_peak_usage() - $startPeak; + + // The DoS angle: bounded memory regardless of whether the parser + // returns a SimpleXMLElement (lenient) or null (strict). + self::assertLessThan(1024 * 1024, $delta, 'parsing must not allocate megabytes from entity expansion'); + + // If the parser DID accept it, the lol3 reference must not have been + // expanded into hundreds of `lol`s. + if ($xml !== null) { + $textContent = (string) $xml->text; + self::assertLessThan(50, strlen($textContent), 'entities must not have expanded'); + } else { + $this->addToAssertionCount(1); + } + } + + public function testParse_GHSA3446_PlainSvgWidthHeightStillParsed(): void + { + $xml = self::safeParseSvg('<svg xmlns="http://www.w3.org/2000/svg" width="42" height="84"><rect/></svg>'); + self::assertNotNull($xml); + self::assertSame('42', (string) $xml->attributes()->width); + self::assertSame('84', (string) $xml->attributes()->height); + } + + public function testParse_GHSA3446_PlainSvgViewBoxStillParsed(): void + { + $xml = self::safeParseSvg('<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 200 150"><rect/></svg>'); + self::assertNotNull($xml); + self::assertSame('0 0 200 150', (string) $xml->attributes()->viewBox); + } +}
tests/unit/Grav/Common/Security/ZipSlipSecurityTest.php+71 −0 added@@ -0,0 +1,71 @@ +<?php + +use Grav\Common\GPM\Installer; + +/** + * Class ZipSlipSecurityTest + * + * Covers: GHSA-w48r-jppp-rcfw (malicious plugin/theme ZIP via directInstall). + * The unZip path now pre-validates every entry name and aborts the install + * if any look like Zip Slip primitives — `../` traversal, absolute paths, + * Windows drive letters, NUL bytes, etc. Well-formed entries still extract + * normally. + * + * Note: this test pins the path-layer hardening only. Defending against a + * well-formed but malicious plugin (whose own PHP is the payload) is a + * separate "trust the source" problem the admin owns when using + * directInstall — see the changelog and advisory triage notes. + * + * Naming convention: test{Method}_{GHSA_ID}_{description} + */ +class ZipSlipSecurityTest extends \PHPUnit\Framework\TestCase +{ + /** + * @dataProvider providerGHSAw48r_DangerousEntryNames + */ + public function testIsSafeArchiveEntry_GHSAw48r_RejectsDangerousNames(string $name, string $description): void + { + self::assertFalse(Installer::isSafeArchiveEntry($name), "Should reject: $description"); + } + + public static function providerGHSAw48r_DangerousEntryNames(): array + { + return [ + ['', 'empty entry name'], + ["foo\0bar", 'NUL byte in name'], + ['../etc/passwd', 'classic parent-dir traversal'], + ['plugin/../../etc/passwd', 'embedded traversal'], + ['plugin/sub/../../../etc', 'deep traversal'], + ['/etc/passwd', 'absolute Unix path'], + ['/var/www/html/shell.php', 'absolute web-root path'], + ['\\windows\\system32', 'absolute Windows-style backslash'], + ['C:\\windows\\evil.dll', 'Windows drive letter (backslash)'], + ['c:/windows/evil.dll', 'Windows drive letter (forward-slash, lowercase)'], + ['plugin\\..\\..\\etc', 'Windows-style backslash traversal'], + ['..', 'bare ..'], + ['./../foo', 'mixed ./ and ../'], + ]; + } + + /** + * @dataProvider providerGHSAw48r_SafeEntryNames + */ + public function testIsSafeArchiveEntry_GHSAw48r_AcceptsLegitimateNames(string $name, string $description): void + { + self::assertTrue(Installer::isSafeArchiveEntry($name), "Should accept: $description"); + } + + public static function providerGHSAw48r_SafeEntryNames(): array + { + return [ + ['plugin/', 'plugin folder root'], + ['plugin/plugin.php', 'plugin entry file'], + ['plugin/blueprints.yaml', 'blueprint'], + ['plugin/templates/index.html.twig', 'nested template'], + ['plugin/assets/img/logo-1.svg', 'nested static asset with hyphen'], + ['plugin/sub.dir/file.ext', 'dotted segment that is not ..'], + ['plugin/.hidden', 'dotfile inside plugin'], + ['plugin/CHANGELOG.md', 'all-caps + extension'], + ]; + } +}
c66dfeb5fSecurity 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]); + } + }; + } +}
Vulnerability mechanics
AI mechanics synthesis has not run for this CVE yet.
References
8- github.com/advisories/GHSA-gwfr-jfjf-92vvghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2026-7317ghsaADVISORY
- github.com/getgrav/grav/commit/5a12f9be8314682c8713e569e330f11805d0a663ghsaWEB
- github.com/getgrav/grav/commit/c66dfeb5fnvdWEB
- github.com/getgrav/grav/security/advisories/GHSA-gwfr-jfjf-92vvnvdWEB
- vuldb.com/submit/798732nvdWEB
- vuldb.com/vuln/359965nvdWEB
- vuldb.com/vuln/359965/ctinvdWEB
News mentions
0No linked articles in our index yet.