VYPR
Medium severity5.0NVD Advisory· Published Apr 28, 2026· Updated Apr 29, 2026

CVE-2026-7317

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.

PackageAffected versionsPatched versions
getgrav/gravPackagist
< 2.0.0-beta.22.0.0-beta.2

Affected products

1

Patches

2
5a12f9be8314

Security fixes: XSS detection, SVG XXE, Zip Slip, media attribute

https://github.com/getgrav/gravAndy MillerApr 23, 2026via ghsa
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 `![alt](img.gif?attribute=onload,alert(1))` 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'],
    +        ];
    +    }
    +}
    
c66dfeb5f

Security fixes: cache + serialization integrity, git-clone shell escape

https://github.com/getgrav/gravAndy MillerApr 23, 2026via ghsa
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

News mentions

0

No linked articles in our index yet.