VDB
Sign up
HIGH8.7

GHSA-vfmf-q6x9-cw96

Grav: detectXss() misses an event-handler attribute after an unpaired quote in an unquoted attribute value, giving stored XSS

Quick fix

GHSA-vfmf-q6x9-cw96 — getgrav/grav: upgrade to the fixed version with the command below.

composer require getgrav/grav:^2.0.15

Details

## Affected versions and vulnerable location

- Confirmed on grav core at `78ebfc1` (tag 2.0.13). - Detector: `system/src/Grav/Common/Security.php:290`, the `on_events` regex, run via `patternMatches()` (`:315-330`). - The `on_events` pattern at HEAD: `#<(?:"[^"]*"|'[^']*'|[^>"'])*?(?:[\s\x00-\x20"'/]|"[^"]*"|'[^']*')on\s*[a-z]+\s*=#iu` - Sole save-time guard for non-super content: `Validation::checkSafety()` (`system/src/Grav/Common/Data/Validation.php:160` scalars, `:165` arrays), invoked per field from `BlueprintSchema::validate` -> `Validation::checkSafety` (`system/src/Grav/Common/Data/BlueprintSchema.php:248`). `security.xss_whitelist: [admin.super]` exempts only super-admins (`Validation.php:148`).

## Root cause (distinct from GHSA-269c)

GHSA-269c hardened the tag-body scan to be quote-aware so a `>` inside a paired quoted attribute value is treated as data, not a tag close. That same quote-awareness opened a new gap: the regex treats ANY `"` or `'` as a string delimiter, but HTML only enters a quoted-value state when a quote appears immediately after `=`. A single unpaired quote sitting inside an unquoted attribute value is, to the browser, just a value character; to the regex it is an unterminated string that neither `[^>"']` nor `"[^"]*"` can consume, so the lazy tag-body scan cannot advance past it to reach the following ` on...=` handler. No alignment matches and `detectXss()` returns null.

## Proof (executed)

The detector was replicated verbatim (the `on_events` regex plus `patternMatches`) with the shipped `system/config/security.yaml` defaults and run under PHP. Observed:

```text baseline <img src=x onerror=alert(1)> => blocked (on_events) GHSA-269c <img src=x title=">" onerror=alert(1)> => blocked (on_events) # prior fix works BYPASS A <img src=x" onerror=alert(1)> => PASSES (no XSS detected) BYPASS B <img title=x" onerror=alert(1)> => PASSES (no XSS detected) BYPASS C <a href=x" onmouseover=alert(1)>x</a> => PASSES (no XSS detected) BYPASS D <img src=x' onerror=alert(1)> => PASSES (no XSS detected) BYPASS E <div id=x" onmouseover=alert(1)>hover</div> => PASSES (no XSS detected) ```

Browser tokenization of `<img src=x" onerror=alert(1)>`: `src` takes the unquoted value `x"` (space ends it), `onerror` is parsed as a separate live attribute, `src` 404s and `onerror` fires. None of the other rules cover it: `img`/`a`/`div` are not in `xss_dangerous_tags`, there is no `javascript:`/`data:` scheme and no `style`/`url`/`expression`.

## Reachability

`checkSafety()` is the only save-time XSS screen for a non-super editor. The pages blueprint validates `header.title` (`type: text`) and page `content` (markdown/textarea) with `xss_check` on, so a bare `onerror=` is rejected but the payload above is stored verbatim. Page content is emitted through `{{ page.content|raw }}` and raw inline HTML passes Parsedown by default (`markdown.escape_markup: false`), so the handler runs for every visitor. The same detector core also backs `Security::detectXssInEditorContent()` (the GHSA-2c4f render-time-Twig save gate; callers `Page.php:1359`, `Flex/Types/Pages/PageObject.php:194`), the `detectXssFromPages()` admin scanner (`Admin.php:2096`), and the `xss()` Twig function, so all of them report the payload clean.

Auth required: an authenticated content editor with page/form edit rights but WITHOUT `admin.super`.

## Suggested fix

Make the tag-body scan treat a quote as a delimiter only in the after-`=` position, or normalize unquoted attribute values before the handler scan, so an unpaired quote inside an unquoted value cannot mask a following `on...=`. A targeted addition: also flag ` on<name>=` sequences that appear after a lone unbalanced quote within the same tag. Because `detectXss()` is a denylist, consider additionally encoding `"`/`'` in stored non-super content, or defaulting `markdown.escape_markup: true` for non-super authors.

## Severity and CVSS reasoning

Suggested severity: High (matches GHSA-269c and the other stored-XSS advisories in this codebase).

Suggested CVSS:3.1 vector: `CVSS:3.1/AV:N/AC:L/PR:L/UI:R/S:C/C:H/I:H/A:N` (8.7).

- `PR:L`: a non-super editor account. - `UI:R`: a visitor renders the page (for `onerror` the image simply loads/fails automatically). - `S:C/C:H/I:H`: script in the site origin against every visitor, including admins viewing the content.

## How I found it and a note on tooling

I read `detectXss()` and reasoned about the difference between the HTML tokenizer's quoted-value state and the regex's string handling, then replicated the exact `on_events` pattern and `patternMatches()` with the shipped default config and ran the payloads to confirm the bypass and that the GHSA-269c payload is still blocked. I used AI assistance for the analysis and drafting and verified the detector behavior by execution. This is executed against a faithful replica of the detector with production config, not against a full running Grav site.

Are you affected?

Enter the version of the package you're using.

Affected packages

Packagist/getgrav/grav
Introduced in: 0Fixed in: 2.0.15
Fixcomposer require getgrav/grav:^2.0.15

References