Files
WP-LIT-MCP/docs/superpowers/notes/2026-10-19-etappe-4e-security-audit.md
T

294 lines
39 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Etappe 4 (Designsystem: Presets, Zuweisen, Farben, Variablen, Theme-Optionen, Schriften, Schleifen): security audit
- **Scope:** branch etappe-4, `d391cb5^..5220876` (HEAD, 153 commits; `d391cb5` is the Etappe 4 spec, so this is the whole Etappe 4 and not only `d73144a^..HEAD`, which starts at plan 4c and would miss 4a/4b). 177 `src/` files changed. Focus per spec §12 and plan task 8:
- (a) preset `attrs`/`from_node`;
- (b) font upload and `RemoteFileFetcher`;
- (c) mass writes (`PageRewriter`, `lit/divi-presets-assign`, `lit/divi-design-detach`, P8/P8a, `HARD_CAP`, `post_ids`);
- (d) loop options (type boundary, S29/S30 for `loop_*_meta_key_`, X1 context, `StoredNodeGuard`, move warning, Z2);
- (e) snapshot restore (rights per area, `MAX_BYTES`, FIFO, `DesignLock`, ID parsing, restoring stored data);
- (f) theme option whitelist and sanitizers, `et_update_option`, color and variable adapters, `ItemKeys`/`GroupKeys`, rights in `permission()` and `execute()`, release gate, error oracles.
- **Environment:** DDEV (WordPress 7.1.3, PHP 8.4, Divi 5.15.1), single site, one user (`administrator`), `DISALLOW_UNFILTERED_HTML` undefined, `et_pb_role_settings` empty (Divi defaults). Plugins `postlos` and `asynclistry` inactive before and after (left as found).
- **Method:**
- Read every new or changed `src/` file of the range in five slices (loops/dynamic content, fonts/fetch, presets/rewriter, snapshots/options/colors/variables, registrar/gate) and traced each write route to its checks.
- Read the Divi 5.15.1 code wherever the plugin relies on Divi behaviour: `LoopUtils` (query args, `parse_and_duplicate_loop_blocks`, `_create_duplicated_block`, no-results path), `DynamicContentLoopOptions`, `DynamicContentOptionLoopPostMetaKey`, `DynamicContentACFUtils` (user meta), `DynamicData`/`DynamicContentUtils` (loop_id passing), `BlockParser` (global layout expansion), `GlobalData` (colors, variables, sanitizers), `FrontEnd/Module/Style.php` (variable CSS output), `epanel/custom_functions.php` (`et_update_option`), `PageResource`, `LoopPresetAttrsMap`, `GlobalPreset::prepare_data`, font upload (`et_pb_add_font`, `check_filetype_and_ext_font`); core `class-wp-query.php` (private status handling), `option.php` (`add_option`), Requests redirect hooks.
- Ran read-only `wp eval-file` probes. Probe files were deleted afterwards. Covered: `et_divi` layout and key count, Divi role defaults for an in-memory editor, snapshot counts, the live loop option registry (34 options), `RemoteFileFetcher::allows()` against 20 host tricks, and `LoopUtils::parse_and_duplicate_loop_blocks` on in-memory block markup with an in-memory `get_post_metadata` marker filter (M1, L2).
- Simulated roles with in-memory `WP_User` objects (never saved): editor without `edit_theme_options`, administrator without `unfiltered_html`, without `upload_files`, without the Divi role options `custom_fonts_management`/`variables_manager`/`theme_customizer`.
- Nothing was written to the site. md5 before and after, identical: `et_divi` 771d6532…, `et_divi_global_variables` 191a4ce9…, `et_uploaded_fonts` a2072c8a… (empty), `et_divi_builder_global_presets_d5` 53586deb…, content hash of all pages/posts/library/TB posts f28463a0…; the `lit_mcp_snapshots_*` options unchanged. `git status` clean except this note.
- `vendor/bin/phpunit`: 3449 tests, 21498 assertions, OK.
**Result:** no Critical or High findings. One Medium, nine Low, ten Info.
- M1 is the one finding that defeats the type boundary of the Global Constraints silently: an empty inner loop hands its `loop_post_meta_*` content to an outer term/user loop.
- The ruling Z2 (two-call route stays a warning) is acceptable only together with M1 and L2: today the warning is wrong for alias/case query types (L2), and M1 reaches the same result with no warning at all.
- Ruling X7(d) (presets enabling loops) turned out harmless on render: Divi's loop pass reads raw block attrs, never presets. A preset-based loop switch is therefore not a finding.
- W13 items hold as recorded except two: the `et_update_option` lost update also happens between two LIT tools (L5), and `DesignLock` is not atomic at all, not only racy on takeover (L6).
---
## Findings (most severe first)
### M1 – Medium (type boundary bypass): an empty inner loop lets the outer loop of another kind resolve `loop_post_meta_*`, with no refusal and no warning
**Where**
- `src/Integrations/Divi/Compose/LoopContext.php:171-181` (`fromChain()`: the innermost enabled loop wins) and `src/Integrations/Divi/Compose/DynamicContentRules.php:142-165` (checks only that innermost scope).
- Divi `includes/builder-5/server/Packages/Module/Options/Loop/LoopUtils.php:4451-4456`: a loop with no results returns its block unchanged, still holding its `$variable(…)$` references.
- `LoopUtils.php:4572-4598`: the outer loop then runs `replace_loop_variables_in_attrs` / `process_loop_block_inner_blocks` with its own `query_type`/`query_item` over the nested block.
- `DynamicContentLoopOptions.php:150-158`: post meta is read with `object_id = loop_id`, which is the term (or user) ID.
**Scenario**
1. A page has a term loop (built in the Visual Builder, or switched on through `attrs`, X7a).
2. Call 1: `lit-divi-insert` puts a Text module inside it with `attrs` `module.advanced.loop` = `{enable: on, queryType: post_types}` and a query that never matches (`includeSpecificPosts: [999999]`, a large offset, a type without posts). No loop content is in the call, so it passes.
3. Call 2: `lit-divi-update` sets the module's text to `loop_post_meta_key_manual_custom_field` with any `loop_meta_key` that passes S29/S30 (no leading `_`). The node's own scope is post, so `fits()` holds and no warning is produced.
4. On the frontend the inner loop has no results; the outer term loop resolves the variable: the meta of the post whose ID equals each term ID, whatever its status or type (drafts, private posts, ACF fields of other types).
5. The account needs only `edit_post` on the page and dynamic-content rights. It is the result the Global Constraints call "keine Komfortmeldung", and unlike Z2 nothing tells the user.
**Evidence:** probe (in memory): `LoopUtils::parse_and_duplicate_loop_blocks` inside `the_content` on markup "category term loop > post loop with no hits > text with `loop_post_meta_key_manual_custom_field`", with a `get_post_metadata` filter that marks reads. The inner block kept `__loop_no_results: true`, and the text resolved to the marker for post 1 (= term ID 1). A stack trace showed the read coming from the outer loop's `_create_duplicated_block` (`:4591`).
**Fix (smallest)**
- For meta-key options only (`str_contains($name, '_meta_key_')`), require every enabled loop in the ancestor chain to have the option's kind, not only the innermost one.
- `LoopContext::fromChain()` collects the kinds of all enabled ancestors-or-self into `LoopScope` (e.g. `kinds(): list<string>`); `LoopScope::fits()` checks meta options against all of them, other `loop_*` options keep the innermost rule (their Divi resolvers check `instanceof WP_Post`/`WP_Term`/`WP_User` and return '' for a foreign object, `LoopUtils.php:3162+`, `3553+`, `3599+`).
- The same chain rule must hold in the update/move warnings (`DiviWriteAbility::loopWarning()`).
- Tests: post meta option in a post loop nested in a term loop is refused on insert and update; in a post loop nested in a post loop it is accepted.
### L1 – Low (disclosure, Divi parity): loop settings through `attrs` let any account with `edit_post` publish private posts of others and user e-mails/logins
**Where**
- `src/Integrations/Divi/Compose/ValueCleaner.php:63-64`: `module.advanced.loop` is not in the builder-only paths, so any writer may set it (X7a; test `DiviLoopWriteTest.php:206-228`).
- Divi `LoopUtils.php:1005-1037`: an empty `subTypes`, or one whose types are all invalid, falls back to `post_type = any`; `any` or `attachment` sets `post_status = [publish, inherit, private]`.
- Core `wp-includes/class-wp-query.php:2685-2687`: without `perm => readable`, explicit `private` is merged into the visible statuses for every visitor. Divi never sets `perm`.
- Divi `LoopUtils.php:922-990`: the user query has no capability check, `role__in` is free.
**Scenario**
1. An author with dynamic-content rights builds a post loop with empty `subTypes` on their own page and uses `loop_post_title`/`loop_post_excerpt`. The published page lists other authors' private posts, although the author lacks `read_private_posts`.
2. Or a user loop for role `administrator` with `loop_user_email`/`loop_user_username`: the page publishes admin e-mails and logins; `list_users` is bypassed. A contributor sees the same in preview.
3. The Visual Builder allows the same to any account with builder access, so this is Divi parity. LIT adds no new effect but opens it to every MCP account with `edit_post`, while the guide says "Abfrage und Anzahl gehören in den Divi-Builder".
**Evidence:** by reading Divi and core (paths above). No private posts or second account exist on DDEV, so no end-to-end probe was run.
**Fix (product decision, behaviour change)**
- Option A (one line, also closes step 2 of M1 and the Z2 update route for lower roles): add `advanced\.loop` to `ValueCleaner::BUILDER_ONLY` (needs `unfiltered_html`), like the other builder-only paths.
- Option B (narrower): refuse a loop with `queryType` `user_roles` without `list_users`, and a post loop whose `subTypes` is empty, `any` or contains `attachment` without `read_private_posts`.
- Recommendation: B, because it keeps loop editing for editors and closes only the two disclosures.
### L2 – Low (wrong warning, normalisation): `LoopScope` reads `queryType` raw, but Divi `sanitize_key()`s it and knows the aliases `terms`/`users`
**Where**
- `src/Integrations/Divi/Compose/LoopScope.php:32-38`, `:76-79`; `LoopContext.php:162-165`.
- Divi `LoopUtils.php:729-731` (`sanitize_key`), `:881-898` (`users` runs the user query, `terms` the terms query), `:4111-4128` (loop_id = term_id / user ID).
- `src/Integrations/Divi/Pages/DiviWriteAbility.php:141-160` (warning text), `DiviUpdateAbility.php:207` (none → unsupported: both kinds null, no warning).
**Scenario**
1. For LIT, `"Post_Taxonomies"`, `" post_taxonomies"`, `"terms"` and `"users"` are unsupported, so new `loop_*` content under them is refused (fail closed, good).
2. Z2 route: post loop with `loop_post_meta_*` children, then `lit-divi-update` sets `queryType` to `terms` or `Post_Taxonomies`. The warning says the content is "jetzt außerhalb jeder Divi-Schleife; Divi zeigt sie dort leer".
3. Divi in fact runs a term loop and prints post meta by term ID. The warning ruling Z2 relies on misleads the user into thinking the change is harmless.
**Evidence:** probes "queryType `terms`, flat" and "`Post_Taxonomies` > empty post loop" both resolved the post 1 marker (term ID 1).
**Fix**
- In `LoopContext::ownScope()`, normalise like `sanitize_key`: `preg_replace('/[^a-z0-9_\-]/', '', strtolower($q))`.
- Add `'terms' => TERM` and `'users' => USER` to `LoopScope::KINDS`.
- In the update/move warning, word an unsupported target as "kann Daten eines anderen Objekts zeigen", not "leer".
- Tests: `Post_Taxonomies` and `terms` are term scope; the update warning names the term kind.
### L3 – Low (type boundary, no check or warning): global instances and copies carry `loop_*` content across loop kinds
**Where**
- `src/Integrations/Divi/Library/LibraryInsertAbility.php:152-163`: in mode `global`, `StoredNodeGuard` sees only the instance's local attrs; the entry content is referenced and Divi expands it before the loop pass (`BlockParser.php:917-924`, then `:947`).
- `src/Integrations/Divi/Compose/StoredNodeGuard.php:64-66`: no checks at all for `unfiltered_html` accounts.
- `DiviNodeCopyAbility.php:172`, `DiviMoveAbility.php:93`: `LoopContext::optionsIn()` skips opaque nodes (`LoopContext.php:192-194`), so moving a global instance never warns.
**Scenario**
1. A library entry holds `loop_post_meta_key_*` (built in the VB, or saved by an `unfiltered_html` account with `lit/library-save`).
2. Any account with library and page rights inserts it as a global instance into a term loop: post meta by term ID, no warning.
3. `unfiltered_html` accounts can also `lit/divi-node-copy` or `lit/library-insert` (copy mode) loop content into any loop kind, again without the warning that move and update give.
**Fix**
- In `library-insert` (global and copy) and `node-copy`, collect `optionsIn()` of the content that will render (entry blocks or the copy) for `_meta_key_` options and compare with the target's `loopAt()` under M1's chain rule; refuse, or at least add `loopWarning()`, for every account including `unfiltered_html`.
- In move, enter global instances through their entry, or warn whenever an opaque global instance changes loop kind.
### L4 – Low (authorization, P8/P8a): `lit/divi-design-restore` never computes affected pages, so it skips `blocked_ids` and `in_use`
**Where**
- `PresetRestorePlanner.php:64-72`, `ColorRestorePlanner.php:67`, `VariableRestorePlanner.php:54-60`, `ThemeOptionsRestorePlanner.php:62-69` build `DesignPlan(..., usages: null)`.
- So `DesignChange::blocked()` (`DesignChange.php:200-219`) is always null on restore, and the dry run shows `affected: []`.
- Compare the set tools, which run `ReferenceUsageQuery`: `DiviColorsSetAbility.php:202-218`, `DiviVariablesSetAbility.php:179-195`, `DiviPresetsDeleteAbility.php:174-176` (`in_use`), `:196`. Spec §4 step 4 requires `affected`/`blocked_ids` for every design write tool.
**Scenario**
1. An account with `edit_theme_options` and Visual Builder rights lacks `edit_post` on live page X, which uses color C.
2. `lit/divi-colors-set` on C is refused (P8a).
3. The same account restores an older colors snapshot in which C had another value or did not exist: page X changes without a check. The same works for variables, and for presets (restore removes presets in use without `in_use`).
4. Limits: only earlier states are reachable, the account is administrator-level, and Divi's builder lets it change globals anyway.
**Fix:** in each planner except theme options, collect IDs whose diff action is `changed`, `removed` or `deactivated` and pass `$usages->reportFor(new ReferenceUsageQuery($ids))` (presets: `PresetUsageQuery::withGroups`) as the fourth `DesignPlan` argument; inject `GlobalUsages` through the constructors (`DiviIntegration`). Keep the preset orphan warning (spec §5) but let P8a block. Test: a colors restore that changes a color used on an uneditable live page answers `forbidden` with `blocked_ids`.
### L5 – Low (integrity, W13 extended): colors and theme options take different locks but both rewrite the whole `et_divi` array from Divi's request global
**Where**
- Divi `GlobalData::set_global_colors()` (`GlobalData.php:682-727`) and `ThemeOptionsAdapter::write()` (`ThemeOptionsAdapter.php:398-403`) both end in `et_update_option()`, which writes `$GLOBALS['et_theme_options']`, loaded once per request (`epanel/custom_functions.php:276-312`). Probe: `et_divi` has 153 keys, `et_global_data` lives inside it.
- `DesignLock.php:184-191`: `lit_mcp_lock_colors` and `lit_mcp_lock_theme_options` are different locks; each `design_rev` covers only its own keys.
**Scenario:** a confirmed `lit/divi-colors-set` and a confirmed `lit/divi-theme-options-update` overlap, or the theme options request loaded `et_divi` before the colors write. The later `et_update_option` writes its stale copy back and silently reverts the global and customizer colors. Both read-backs pass because each checks only its own keys; the colors snapshot and log describe a state that is gone. Same principal, integrity only.
**Fix:** map `colors` and `theme_options` to one lock name in `DesignLock::option()`, and under the lock refresh `$GLOBALS['et_theme_options'] = get_option('et_divi');` before writing (`ThemeOptionsAdapter::write()`, `GlobalColorsAdapter::save()`).
### L6 – Low (concurrency): `DesignLock::acquire()` is not atomic, and a stale takeover lets the old holder release the new holder's lock
**Where**
- `DesignLock.php:172-176` (docblock "add_option() is atomic") and `:193-207`.
- Core `add_option()` (`wp-includes/option.php:1119-1125`, `:1142`) checks with `get_option` first and then runs `INSERT … ON DUPLICATE KEY UPDATE`; two concurrent requests can both get `true`.
- The plugin already knows the safe pattern: `src/Integrations/Divi/Release/DiviCheckRoute.php:19-22` (`INSERT IGNORE` plus compare-and-swap).
- Stale takeover after 120 s: a long confirmed run (`PageRewriter` with 50 pages) loses its lock to B; A's `finally release()` (`DesignChange.php:97-99`) deletes B's lock, so C enters too.
**Scenario:** two confirmed changes of one area both pass `design_rev` and write; last write wins, and snapshots and change log disagree with the stored state. Same principal, integrity only. This also widens the recorded `attachLog`/RMW and takeover items.
**Fix:** reuse `DiviCheckRoute`'s pattern: `INSERT IGNORE` with a token `time() . ':' . wp_generate_password(8, false)`, success only on `rows_affected === 1`; take over a stale lock with `UPDATE … WHERE option_value = <read value>`; release only while the token still matches.
### L7 – Low (CSS injection without `unfiltered_html`): image variable values can leave the `:root` rule
**Where**
- `src/Integrations/Divi/Design/VariableValue.php:36` (`WEB_URL = /^https?:\/\/[^\s<>"]+$/iD`) allows `; { } ( ) '`.
- Divi `sanitize_global_variables_data` applies `sanitize_text_field` to images (`GlobalData.php:836-878`), which keeps them.
- Divi `FrontEnd/Module/Style.php:1503-1505` wraps images as `"url({$result})"` without escaping, inside `:root{--gvid-…: …;}`.
- `numbers` and `fonts` already exclude `;{}<>` (`VariableValue.php:62`, `:101`); `images` is the gap.
**Scenario:** an administrator without `unfiltered_html` (multisite site admin, or `DISALLOW_UNFILTERED_HTML`) holding `variables_manager` sets an image variable to `https://x.test/a.png);}body{display:none}a{b:url(x`. Every page that uses the variable gets the injected CSS (hide content, overlays, tracking `url()`). No `<`, so no `</style>` and no script. Spec §4 step 2 reserves raw CSS for `unfiltered_html`. Divi's own variable manager accepts the same value (parity), hence Low.
**Evidence:** `preg_match(WEB_URL, 'https://x.test/a.png);}body{display:none}a{b:url(x')` returns 1; the Divi output line was read.
**Fix:** `WEB_URL = '/^https?:\/\/[^\s<>"\'(){};\\\\]+$/iD'` (and the same for a `/`-relative branch if images accept it). Test: the value above is refused.
### L8 – Low (SSRF, transport): the IPv4-only rule and the redirect checks around cURL do not apply to the fsockopen transport
**Where:** `src/Integrations/Media/RemoteFileFetcher.php:59-64`, `:72`. The `http_api_curl` hook does not fire on the Requests fsockopen transport; `PublicAddress::dnsResolver()` and `wp_http_validate_url` check IPv4 only.
**Scenario:** on a host without ext-curl, `stream_socket_client` may connect over AAAA. A name with a public A record and an AAAA record of `::1` or `fd00::…` reaches an internal IPv6 target (fonts: blind oracle only; media import: an image/PDF answer becomes a readable attachment). Needs a server without cURL, which is rare; DDEV has cURL.
**Fix:** in `fetch()`, return `failed(…)` when `! function_exists('curl_init')`, or restrict this request to cURL with a scoped `http_api_transports` filter (`fn () => ['curl']`).
### L9 – Low (DoS): the page rewriter parses every candidate post on each dry run and confirm, with no cap on drafts
**Where**
- `src/Integrations/Divi/Library/GlobalUsages.php:101-116` (`rowsFor()`: up to `PARSE_CAP` 5000 parsed, the rest appended).
- `src/Integrations/Divi/Design/PageRewriter.php:184-216` (`changes()`: `loadWritable()`, parse and `TreeRewrite` for every row, for `preview()` and `apply()`, beyond `PAGE_LIMIT` 50 because the remainder is counted); `:89`, `:140` (`reportFor()` first, so candidates are scanned twice).
- `HARD_CAP` (2000) counts live posts only.
**Scenario:** a site with thousands of drafts holding `divi/text`; every `lit/divi-presets-assign module=text` dry run parses them all, up to `max_execution_time`. Memory stays bounded by chunks of 50. A timeout during `apply()` keeps the posts saved so far (each with a revision) but returns no `partial_write`. Administrator-level actor, effect ends with the requests.
**Fix:** stop the in-memory rewrite after `PAGE_LIMIT` changed posts plus a bounded counting window (e.g. 500 candidates) and report `remaining` as "mindestens N"; or refuse above a total candidate cap (all statuses) unless `post_ids` is given. Test: 600 candidate rows load at most `PAGE_LIMIT` plus the window.
---
## Info / hardening
### I1 – Preset-side variable detach inlines `before`/`after` without the marker check of the page-side detach
- `PresetBook.php:366-378` (`resolved()` concatenates affixes raw) vs `VariableDetachRewrite::hasUnsafeAffix()`/`unsafe()` (`VariableDetachRewrite.php:41-45`, `120-147`).
- `markupOutside([])` (`DiviDesignDetachAbility.php:429`) and Divi's `prepare_data()` catch `<`/`>`, but a `$variable(…)$` stored in an affix by an `unfiltered_html` account becomes a live, never-canonicalised reference once inlined. Fix (about 5 lines): skip or refuse a reference whose affixes are `unsafe($text, markupAllowed: true)` and list it in the "eingebettet" note.
### I2 – A detached variable value is inlined without cleaning for the target field type
- `VariableDetachRewrite::replacement()` (`:51-61`); only `<`/`>` and markers are checked (`DiviDesignDetachAbility.php:277-291`). A strings value `javascript:…` lands raw in a link field. Divi escapes `href`/`src` with `esc_url` on render (`HTMLUtility.php:791-793`, `1054-1068`), so not exploitable. Optional: `ValueSanitizer::url()` when `kindAt()` is Url.
### I3 – For `unfiltered_html` accounts, `from_node` copies `loop_*`/`custom_meta_*` references unchecked into a preset
- `PresetComposer.php:75-82`; `StoredNodeGuard::problems()` returns `[]` for them (`StoredNodeGuard.php:64-66`). The `attrs` path refuses every `loop_*` (no scope). A preset can then carry a loop reference into modules of any loop kind. Administrator actor. Optional: run the taken attrs through the composition's `references()` for the loop rule regardless of `unfiltered_html`.
### I4 – Read tools check rights only in `permission()`
- `DiviPresetGetAbility.php:104-109`, `DiviDesignSystemAbility`, `DiviDesignSnapshotsAbility`. `AbilityRegistrar.php:66-82` always runs `permission_callback` first, so unreachable; inconsistent with the write tools and `DiviThemeOptionsGetAbility.php:114-118`.
### I5 – Shortcodes and custom-field conditions inside term/user/menu loops run against the post with the term/user ID
- Divi `LoopUtils.php:4507-4510` (`__loop_post_id` = term/user ID), `DynamicContentUtils.php:669-690`, `ModuleElements.php:1718-1771` (`override_post_data()` → `get_post(term_id)`), `Conditions.php:928-940`. `[acf]`, `[pods field=…]` and similar in a text module in a term loop read the unrelated post. Outside the `loop_*` check and Divi behaviour; document in the guide. Optional: refuse shortcodes under a non-post loop without `unfiltered_html`.
### I6 – Font dry run reveals whether a host name resolves privately
- `DiviFontsUploadAbility.php:216` ("nur öffentlich erreichbare http(s)-Adressen") vs "nicht abrufbar": internal DNS enumeration for an administrator-level principal. Media import behaved so before Etappe 4.
### I7 – Font confirm is not bound to the dry run
- Confirm downloads again (`DiviFontsUploadAbility.php:288` ff., `download()` `:320`) without comparing the SHA-256 shown in the dry run. The content is re-checked, so nothing unsafe gets in. Optional `expected_sha256` input.
### I8 – Font download time and leftovers
- Up to 4 files × 5 MB × 30 s (`TIMEOUT`, `:24`), both in dry run and confirm; a slow server holds a worker about 120 s per call. If PHP is killed mid-download, `wp_tempnam` files stay (cleanup only in `finally`). Administrator-only.
### I9 – Lost update on `et_uploaded_fonts`
- `font_add` reads and writes the whole option without a lock (fonts have no pipeline, A2). Concurrent uploads, or an upload next to a Divi UI upload, can drop an entry and leave orphan files in `et-fonts`. Same class as W13.
### I10 – `numbers` variables accept `url(https://…)`
- No breakout (`esc_html`'d, `;{}<>` excluded), but a tracking request from every page is possible. Acceptable.
---
## Deferred items re-checked
| Item | Verdict |
|---|---|
| W13 lost update of `et_update_option` | Confirmed, and wider than recorded: also between `colors-set` and `theme-options-update` (L5). **Zu beheben** (L5); against Divi's own saves acceptable. |
| W13 `body_header_style` | One of italic/uppercase/underline only; Divi can combine. Functional, not security. **Akzeptabel.** |
| W13 missing range check in Divi | `ThemeOptionSpec::check()` enforces min/max/step/choices before Divi's sanitizer; a sanitizer change is an error (`DiviThemeOptionsUpdateAbility.php:183-193`). **Akzeptabel.** |
| W13 `font_add` partial success | `FontInstaller::install()` (`:402-450`) compares the formats Divi stored (`FontInstaller.php:122-124`) and calls `font_remove($name)` on a shortfall; the name rules guarantee Divi's stripped name equals `$name`; a failed removal is `WriteFailed::partial` with a hint. **Akzeptabel.** |
| W13 `remove_static_resources` only with a logged-in user | Divi checks `et_core_security_check_passed('edit_posts')` (`PageResource.php:1404`); MCP requests are always authenticated. Only CLI/cron would miss the CSS clear. **Akzeptabel.** |
| W13 tablet/phone 14/30 quirk | Display defaults and inheritance only. **Akzeptabel.** |
| W13 DNS-rebinding window between `allows()` and cURL | Confirmed (`RemoteFileFetcher.php:37-42` vs `:51-84`, redirects `:67-71`; nothing pins the checked IP; ports 80/443/8080). For fonts a blind boolean oracle only (magic bytes + finfo, nothing stored): Low. For media import (authors with `upload_files`) an internal image/PDF answer becomes a readable attachment; that is the 3a/3b "DNS pinning" item, carried over unchanged. **Zu beheben** (about 15 lines): resolve once in `fetch()`, check, and pin with `CURLOPT_RESOLVE` in the existing `http_api_curl` hook, redirect hosts likewise. |
| W13 schema errors without lit field paths | UX only; `ThemeOptionSpec` errors carry `set.<key>`. **Akzeptabel.** |
| Ruling R9 (rewriters without `unfiltered_html`) | No client string enters page content: assign writes preset IDs through `PresetAssignment` → `keepsId`, preset detach removes IDs, variable detach refuses `<`/`>`/markers and unsafe affixes per block without `unfiltered_html`. `DiviSaver`'s kses window keeps existing markup byte-identical. **Akzeptabel.** |
| P8a with `post_ids` | Only named posts are rewritten (`PageRewriter.php:187-189`), each through `loadWritable()` (`edit_post`, Library/TB access and group gate); a named library entry gets its own usage report (`:282-292`). **Akzeptabel.** |
| `SnapshotStore::attachLog` without error check, RMW without lock | Inside the area lock, but the lock is not atomic (L6). Worst case a missing `log_id` link. **Akzeptabel**, resolves with L6. |
| `DesignLock` takeover race | Worse than recorded: `acquire()` itself is not atomic (L6). **Zu beheben** (L6). |
| X7(a) loops switchable through `attrs` | Holds as designed for the type boundary, but enables L1 (private posts, user data). **Produktentscheidung** (L1, options A/B). |
| X7(b) global modules without context | `loop_*` in library entries is refused (no scope). The inbound direction via global instances is L3. **Akzeptabel** for entry edits; L3 zu beheben. |
| X7(c) copying fail closed | Holds for accounts without `unfiltered_html` (`StoredNodeGuard` composes with scope none, every `loop_*` refused). `unfiltered_html` accounts get no check and no warning (L3). **Akzeptabel** with L3. |
| X7(d) presets with loop | No render effect: `LoopUtils::parse_and_duplicate_loop_blocks` parses the raw document with `WP_Block_Parser` and reads raw `attrs` (`LoopUtils.php:4219-4229`, `718-733`); presets never start, stop or retype a loop on the frontend. X1 matches Divi's render. **Akzeptabel** (fail-closed limit only). |
| X7(e) ACF repeater fields | `loop_acf_*` has no kind, `repeater_*` is unsupported, all refused. **Akzeptabel.** |
| Ruling Z2 (two-call route stays a warning) | Impact: non-protected post meta of posts whose IDs equal term/user IDs. Acceptable as Low **only** while the warning is accurate and no silent route exists; today L2 makes the warning wrong and M1 is silent. Recommendation: fix M1 and L2; for `_meta_key_` options refuse (instead of warn) on update/move, ordinary loop editing stays possible. |
| Mirror `customizer-typography` | Fingerprints all of `et_divi_customizer_theme_settings`; any customizer edit locks ThemeOptionRead/Write (fail closed, noisy). **Akzeptabel** as documented shortcut. |
| Ledger lead: `LoopContext::of` skips an opaque node's own loop | Used only by dynamic-sources (a listing); write tools never write onto opaque blocks; `loopAt()` does not skip. Info. **Akzeptabel.** |
| Ledger lead: `kindOfOption` defaults to POST | `loopFieldsFor` needs both `fits()` and a map entry from `loopFields(kind)` (Divi group; meta options by name, matching Divi's own prefix rule, `DynamicContentOptionLoopPostMetaKey.php:334-345`). An unknown name never passes. **Akzeptabel.** |
| Ledger lead: user meta via manual `loop_meta_key` in user loops | Divi's user branch of `get_meta_value_by_type()` requires the **viewer** to hold `manage_users` (non-ACF) or `list_users` (ACF) (`DynamicContentACFUtils.php`). No visitor leak. **Akzeptabel**, documented as parity; the real user exposure is L1. |
| Ledger lead: Divi passes `loop_id`/`loop_query_type`/`loop_object` to every content variable | Non-loop options resolve `post_id` from `$value['post_id'] ?? get_dynamic_content_post_id()` (the page); `VariableRef::canonical()` allows only `name`/`settings`, so `post_id` cannot be injected. Of the non-loop resolvers only the repeater branch of `DynamicContentOptionPostMetaKey` and `get_archive_term_thumbnail_id` (returns 0) read loop data. No crossing; the real crossings are M1 and I5. **Akzeptabel.** |
| Ledger lead: none → unsupported loop switch without warning | Confirmed, but no `loop_*` content can exist under none. The real gap is L2. **Akzeptabel** with L2. |
---
## Areas checked and found sound
- **Rights matrix** (`permission()` and `execute()` both check, unless noted in I4):
| Tool | Checks | Divi features |
|---|---|---|
| `lit/divi-colors-set` | `DesignRights::colors`: `edit_theme_options`, Visual Builder | DesignRead, ColorWrite |
| `lit/divi-variables-set` | `edit_theme_options`, Visual Builder, `variables_manager` | DesignRead, VariableWrite |
| `lit/divi-presets-set`, `-delete`, `-assign` | `DesignRights::presets` (`edit_theme_options`, Visual Builder); `markupOutside()` without `unfiltered_html` | assign/detach also PageRead/Write, Library, Theme Builder |
| `lit/divi-design-detach` | rights per preset or variable | as above |
| `lit/divi-theme-options-update` | `edit_theme_options`, `theme_customizer` | ThemeOptionRead, ThemeOptionWrite |
| `lit/divi-theme-options-get` | `edit_posts` | ThemeOptionRead |
| `lit/divi-design-restore` | the area planner's rights; an invalid ID needs all areas' rights | per area, or all |
| `lit/divi-design-snapshots` | `edit_theme_options` (`permission()` only) | none |
| `lit-divi-design-system`, `lit/divi-preset-get` | `edit_posts` (`permission()` only) | DesignRead (+ Catalog) |
| `lit/divi-fonts-upload` | `upload_files`, `edit_theme_options`, `custom_fonts_management`, fail closed without Divi | FontUpload |
`AbilityRegistrar` runs access, OAuth, the guards (`DiviFeatureGate`) and then `permission()` in `permission_callback`, and the guards again in `execute_callback`. `DesignChange::apply()` asserts the feature before the snapshot, each adapter again before writing, `FontInstaller::install()` before any write. In-memory editor: refused everywhere for lack of `edit_theme_options`, although Divi's defaults grant `theme_customizer`, `variables_manager` and `custom_fonts_management`.
- **Preset attrs (XSS):** `set`/`attrs` go through `Composer::update()` (ValueCleaner S7/S26, URL/HTML rules); `from_node` through `StoredNodeGuard` on the whole subtree; every path must be a Divi preset attribute, so text, links and images are refused (`PresetComposer.php:94-102`). A changed preset holding markup is refused without `unfiltered_html` (`DiviPresetsSetAbility.php:541`, same in delete/detach). Divi's `prepare_data()` sanitizes again (`DesignSystemWriter.php:103`). CSS values without `<` cannot leave the `<style>` block.
- **Mass writes:** usages and P8 in presets-set/delete with `withGroups`; delete refuses `in_use` on explicit usages including drafts; TB layouts need ThemeBuilderAccess (hidden ones read as not found and are never named); blocked library entries refuse (P8a), site-wide entries and layouts without revisions are skipped; `read_post` before naming, one `NO_CHANGE` text for every reason (`PageRewriter.php:60-61`, `230-241`); global instances opaque, only listed in `skipped`. `Serializer` escapes `--` and `<`, so an inlined value cannot close the block comment.
- **`ItemKeys`/`GroupKeys`:** `strval` on numeric keys; groups resolved only against Divi's groupId paths; `PresetBook` casts IDs to string at every lookup and writes with `array_values`; `PresetDiff` merges key lists only. No renumbering of option arrays found.
- **Loop type boundary (bypass attempts, all held):** control characters (stripped by `ControlChars` before `canonical`), case or leading space in names (not `loop_`-prefixed → unknown), JSON `\u` escapes (decoded before the checks), `{{loop:…}}` (through `Placeholders` → `canonical`), same-call loop attrs (Insert refuses on the composed blocks, `DiviInsertAbility.php:107`; Update re-composes in the new scope, `DiviUpdateAbility.php:240-246`), tablet/hover/`true` enable (neither LIT nor Divi counts them), integer/bool `queryType` (unsupported → refused), `loop_position` (changes only `loop_object`; post meta uses `loop_id`, term/user branches check `instanceof`). S29/S30 cover `select_loop_meta_key` and `loop_meta_key`; prefix stripping matches Divi's `get_meta_key_from_name`. Registry unavailable → default map → every `loop_*` refused. Live registry: 34 loop options, groups as assumed, `enable_html` refused.
- **Theme options:** 20 typography and layout keys, no custom CSS or head code; unknown keys refused at write (`ThemeOptionsAdapter.php:393-396`); font names without `<>"'\`` and control characters, exact Divi font required (`FontCheck`).
- **Colors and variables:** colors only hex, keywords or functions without `( ) ;` or quotes; string variables rendered through `wp_kses_post` (`DynamicContentGlobalVariableOptions.php:149-153`); fonts and numbers without breakout characters (images: L7).
- **Snapshots:** strict ID regex `^(colors|variables|presets|theme_options)-\d{8}T\d{6}Z-[0-9a-f]{6}$/D`; area taken from the ID, lookup by area and `===`; not found says nothing about other areas. `MAX_BYTES` 2 MB (413 before writing), FIFO 20, `TOTAL_BYTES` 8 MB keeping the newest, not autoloaded. The list shows `user_id`, `ability`, `log_id`, never `data`; data appears only as a restore diff behind the area's rights. Stored data is not re-validated on restore, but it is written only by `SnapshotStore::save()` from Divi's own inventory and no tool writes `lit_mcp_snapshots_*` (excluded from the settings whitelist), so changing it needs database or option access; partial re-checks still run (presets `markupOutside` both ways, Divi colors/variables sanitizers and `validate_global_variable_ids`, theme option whitelist). Rollback restores the snapshot on failure; a failed restore becomes `partial_write` naming the snapshot.
- **Font upload:** `allows()` denied `2130706433`, `0x7f.1`, `017700000001`, `127.1`, `[::ffff:127.0.0.1]`, `[::ffff:7f00:1]`, `localhost`, `localhost.`, `0`, `[::]`, `[fd00::1]`, `169.254.169.254`, `[64:ff9b::a9fe:a9fe]`, port 22, `ftp://`, `user:pw@`, the site's own host and public IPv6 literals (cURL is IPv4-only). Redirect hook name matches `WP_HTTP_Requests_Hooks::dispatch`; relative locations are absolutised first; both hooks removed in `finally`. Size `limit_response_size = max+1`, `max = min(upload limit, 5 MB)`, at most 4 files, empty files refused. Type: own magic bytes (`FontSignature`) must equal the format, then Divi's finfo check (fails closed without fileinfo), then `wp_handle_upload` with `test_type`. Name `[A-Za-z0-9 _-]`, length per Divi, case-insensitive collision check against Google, web-safe and uploaded fonts before any download; `sanitize_file_name`, Divi's `upload_dir_font`, `wp_unique_filename`: no traversal. Temp files deleted in `finally` on every path. Network reasons go to the log only (URL without credentials, query, fragment). The media import extraction behaves as before Etappe 4.
## Status nach der Fix-Welle (2026-10-20)
Basis: Audit-HEAD `5220876`, Stand danach `8ff3b16` (`composer test`: 3499 Tests grün, `composer lint` sauber; Smokes `divi-loop`, `divi-colors-variables`, `divi-theme-fonts`, `divi-presets` grün).
| Befund | Status |
|---|---|
| M1 | behoben in `522fc8f` (Kettenregel für `_meta_key_`-Optionen), `3a89b15` (Warnung nur bei Inhalten, die vorher passten), `8be2f5e` (Update: Composer und Zweitdurchlauf nach der ganzen Kette, vom Gesamtreview als umgehbar gefunden), `71ffbdb` (Warnung nennt die Kette) |
| L1 | Produktentscheidung Option B, behoben in `4a8a889`, `71c2e03` (Prüfung auf dem bereinigten Wert), `6e84138` (`queryType: null` wie Divi) |
| L2 | behoben in `522fc8f` |
| L3 | **offen**: globale Instanzen und Kopien (`unfiltered_html`) tragen `loop_*`-Inhalt über Schleifen-Arten ohne Prüfung. Eigener kleiner Plan nötig; bis dahin als Grenze zu behandeln. |
| L4 | behoben in `bd80ab1`, `bb54870` (Restore berechnet Fundstellen; P8a `blocked_ids`, `affected` im Probelauf). Bewusst kein `in_use` bei Presets (Spec §5: verwaiste Presets erlaubt, Warnung bleibt). |
| L5 | behoben in `b59a0c0`, `1b3ef55`, `c4ee195`, `99c7aab`, `f754772` (ein Lock für Farben und Theme-Optionen; unter der Sperre Divis Laufzeit-Caches verwerfen, gegen echtes Divi für Farben, Variablen und Presets nachgewiesen; Uninstaller räumt alte Lock-Namen) |
| L6 | behoben in `b59a0c0` |
| L7 | behoben in `06b798a` |
| L8 | teilweise behoben in `20e0216` (kein Abruf ohne cURL). Rest: bei https mit einem cURL ohne SSL kann Requests auf fsockopen ausweichen; selten, **bewusst belassen**. |
| L9 | **bewusst belassen**: Administrator-Niveau, Wirkung endet mit dem Request; ein Cap ändert das Verhalten und ist eine eigene Entscheidung. |
| I1–I10 | unverändert (Hinweise, kein Fix in dieser Welle) |
Rest-Risiken aus „Deferred items re-checked": DNS-Rebinding behoben in `20e0216` (`CURLOPT_RESOLVE`, auch für Weiterleitungsziele); `DesignLock`-Übernahmerennen behoben (L6); Lost Update zwischen Farben und Theme-Optionen behoben (L5); alle übrigen Punkte wie oben als „Akzeptabel" bewertet. Ruling Z2 (Zwei-Aufruf-Weg nur Warnung) hält jetzt, weil M1 und L2 geschlossen sind.
Re-Reviews: Opus-Re-Review (Wellen 1+2) fand L1-Umgehung und unvollständiges L5; Sonnet-Verifikation fand die `queryType: null`-Regression und das Cache-Problem bei Farben/Variablen/Presets. Beide Funde sind behoben. Die Commits `6e84138` und `99c7aab` hat das Gesamtreview gelesen, ohne Befund; L5 ist zusätzlich per Probe gegen echtes Divi belegt. Das Gesamtreview fand außerdem die Umgehung von M1 im Update (behoben in `8be2f5e`).