diff --git a/docs/superpowers/notes/2026-10-19-etappe-4e-security-audit.md b/docs/superpowers/notes/2026-10-19-etappe-4e-security-audit.md new file mode 100644 index 0000000..0a1d07d --- /dev/null +++ b/docs/superpowers/notes/2026-10-19-etappe-4e-security-audit.md @@ -0,0 +1,293 @@ +# 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`); `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 = `; 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 `` 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.`. **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 `