diff --git a/media-key-defensive-audit.md b/media-key-defensive-audit.md new file mode 100644 index 000000000..2d4fc8981 --- /dev/null +++ b/media-key-defensive-audit.md @@ -0,0 +1,1159 @@ +# Defensive Audit — s9e Media-Site Keys (9 Providers) + +**Scope:** Imgur, Google Sheets, Google Drive, SlideShare, JSFiddle, Internet Archive, CodePen, GitHub Gist, Falstad/CircuitJS. +**Sources:** `s9e/xenforo-ext-media-sites` (new "media key" architecture) + `s9e/TextFormatter` (old `MediaEmbed` plugin, for contrast). +**Threat model:** attacker/user-controlled raw post content (BBCode + pasted URLs) **before** XenForo parsing, media matching, and rendering. + +--- + +## 1. Executive Summary + +The single most important fact: **the media key is user-controllable and is interpolated into XenForo templates with NO HTML escaping** (`{$var}` is raw). There are two distinct input paths that produce a key: + +| Path | How the key is formed | Constraint on the key | +| --- | --- | --- | +| **Bare URL** (e.g. `https://codepen.io/user/pen/abc123`) | `Parser::match()` runs per-site regexes → `serializeVars()` | Constrained by the site's match regex (e.g. `user=[-\\\\w]+`, `id=\\\\w+`) | +| **`[MEDIA=provider]key[/MEDIA]` BBCode** | The user types the key directly; `convertMediaTag()` wraps it as `[URL media="provider:key"]` | **Any character except `"` and `[`** — the key is NOT run through `Parser::match()` | + +The `[MEDIA]` path is the high-value one: it lets the user encode **arbitrary `k=v;k2=v2` pairs** (urldecoded at render time) that are then spliced raw into the template. Because `parseVars()` urldecodes each value, the user can inject a literal `"` via `%22`, a `&` via `%26`, a `<` via `%3C`, etc. — characters that the bare-URL regexes would have stripped. + +**Net effect:** from a single `[MEDIA=...]` tag the user can (a) break out of the iframe's `src`/`style`/other double-quoted attributes, (b) inject new attributes (`onmouseover`, `src`, `style`, `data-*`, …), (c) override the renderer's fixed mechanics (appearance, UI, simulation, loading, external URLs), and (d) reach the `replaceIframes` JSON sink, which re-emits those attributes via `setAttribute` on the client. + +The "CircuitJS" sink the user asked about is the **Falstad** provider: its `src` is `https://www.falstad.com/circuit/circuitjs.html?running=false&cct=…` (or `ctz=…`), and the `cct`/`ctz` value is user-controlled, so the user can (i) choose which circuit is loaded (simulation behavior) and (ii) inject extra query params (e.g. `&running=true`) to override the renderer's fixed `running=false`. + +--- + +## 2. Architecture & Data Flow + +``` +Raw post content (attacker-controlled) + │ + ├─ bare URL ─────────────► Parser::match($url, …) + │ │ per-site regexes (static::$sites[siteId][0]) + │ │ scrape configs (static::$sites[siteId][2]) + │ │ filters (static::$sites[siteId][3]) + │ │ adjustVars{SiteId}() + │ └► serializeVars($vars, $siteId) ──► KEY + │ + └─ [MEDIA=prov]key[/MEDIA] ► convertMediaTag() ──► [URL media="prov:key"] + (key = [^"\[]++ , NOT matched) KEY + │ + ▼ +Html::renderTagUrl() → splits "prov:key" → renderTagMedia([key], prov, …) + │ + ▼ +Renderer::render($mediaKey, $site, $siteId) + │ parseVars($mediaKey): + │ if key =~ ^(\w+=[^;]*)(?:;(?1))*$ → explode ';' → k=v (both urldecode) + │ else → $vars['id'] = $mediaKey + │ + defaultValues (internetarchive: width=640,height=360) + │ + adjustVars{SiteId}() + │ + filters (internetarchive: filterUint width/height) + ▼ +templater->renderTemplate('public:_media_site_embed_{siteId}', $vars) + │ {$var} = RAW (no escaping) + │ {{ expr }} = RAW expression + ▼ +renderTagMedia() wraps in + │ + ▼ +Helper::replaceIframes() (post-processing) + │ regex-parses the iframe's attr="value" pairs + │ re-serializes into data-s9e-mediaembed-iframe='[JSON]' + ▼ +Client JS: JSON.parse(attr) → iframe.setAttribute(name, value) for each pair +``` + +### Key deserialization (`Renderer::parseVars`) +```php +if (preg_match('(^(\w+=[^;]*)(?:;(?1))*$)', $mediaKey)) { + foreach (explode(';', $mediaKey) as $pair) { + list($k, $v) = explode('=', $pair); + $vars[urldecode($k)] = urldecode($v); // ← urldecode = the injection vector + } +} else { + $vars['id'] = $mediaKey; +} +``` + +### Key serialization (`Parser::serializeVars`) +```php +ksort($vars); +if ($keys === ['id'] && preg_match('(^[-./:\w]+$)D', $vars['id'])) return $vars['id']; +// else: urlencode(k)=urlencode(v);… with '..' → '%2E%2E' +``` + +### The `replaceIframe` JSON sink (`Helper::replaceIframe`) +```php +preg_match_all('(([-\w]++)="([^"]*+))', $original, $m); // captures attr="value" +$attributes = array_combine($m[1], $m[2]); // duplicate attr → LAST wins +… +$values[] = $attrName; $values[] = $attrValue; // for every non-c2l attr +$html .= " data-s9e-mediaembed-iframe='" + . str_replace("'", ''', htmlspecialchars(json_encode($values), ENT_NOQUOTES, …)) + . "'"; +``` +Client side: `for(;++b **Consequence:** the `replaceIframes` step does NOT neutralize attribute breakout — it *preserves* it. The regex `attr="[^"]*"` is exactly what turns a `"`-breakout into a new, cleanly-captured attribute. + +--- + +## 3. The Media Key as User-Controlled Input + +### 3.1 The `[MEDIA]` path (primary vector) +`convertMediaTag()`: +```php +if (preg_match('(^\\[MEDIA=(\\w++)\\]([^"\\[]++)\\[/MEDIA\\])i', $markup, $m)) { + $markup = '[URL media="' . $m[1] . ':' . $m[2] . '"]' . $url . '[/URL]'; +} +``` +- `$m[2]` (the key) = `[^"\[]++` → **any character except `"` and `[`**. +- The key is **not** validated against the site's match regex. +- At render, `parseVars()` urldecodes each `v`, so `%22`→`"`, `%26`→`&`, `%3C`→`<`, `%3E`→`>`, `%3D`→`=`, etc. + +**Minimal attribute-breakout primitive** (works on any provider whose template puts a var inside a double-quoted attribute): +``` +[MEDIA=codepen]user=x%22onmouseover%3D%22alert(1)//;id=abc123[/MEDIA] +``` +→ `user = x"onmouseover="alert(1)//` +→ template emits: +```html +