# Etappe 3a (Divi-Bibliothek): security audit

- **Scope:** branch etappe-3, cfc81dd..186f7af. Focus: what the per-task reviews covered least (object authorization, global edits, stored-content injection, disclosure, DoS).
- **Environment:** DDEV (WordPress 7.1.x, Divi 5.2.1), single site, no editors/authors present, `DISALLOW_UNFILTERED_HTML` off.
- **Method:** read all new/changed `src/` files of the range plus the relevant Divi 5 render code (`BlockParser`, `BlockParserStore`, `GlobalLayout`, `DynamicContentOptions*`, `LinkUtils`, MultiView, Divi 4 `class-et-builder-element.php`), scanned all `script-library-*.js` for DOM sinks, and ran four self-cleaning `wp eval-file` probes (temp posts 1906–1914, temp user `litaudit_author`; all deleted, verified: 0 `LITAUDIT` posts, 0 postmeta/term rows for those IDs, no temp user, no probe files, `git status` clean). No [Fixture]/[Test] content was touched; no code was changed.

**Result:** no Critical, High or Medium findings. The rulings P3/P5/P8a/P11/P12/P15–P19 hold. Six Low findings and three Info/hardening notes follow.

---

## Findings (most severe first)

### L1 – Low (DoS): the P8a check loads every live usage with full content, and each dry run scans `wp_posts` 3×

**Where**
- `src/Integrations/Divi/Library/GlobalUsages.php:68-79` (`liveIds()`: no LIMIT).
- `src/Integrations/Divi/Library/GlobalEdits.php:158-171` (`_prime_post_caches($live, …)`, then `current_user_can('edit_post')` per ID).
- `GlobalUsages.php:29-47`: COUNT plus SELECT, each an unindexed `post_content LIKE '%…%'`.
- Also reached from `GlobalEdits::notice()` (every delete dry run) and `LibraryListAbility::detail()`.

**Scenario**
1. A global module (newsletter box, CTA) sits in thousands of posts. That is a common Divi pattern.
2. Any user with library rights and `edit_post` on the entry calls a Divi tool on it without `confirm` (dry run), or `lit/library-list` with `id`.
3. `_prime_post_caches()` fetches `posts.*` for every live ID in one query, `post_content` included. At 5,000 posts × ~50 KB of Divi markup that is about 250 MB, which exceeds the usual `memory_limit` and ends the request in a fatal error.
4. Each call also runs 3 full-table LIKE scans (6 on confirm, because dry run and confirm both call `of()` and `blocked()`). A loop of calls ties up the database on large sites.
5. The effect stops when the requests stop, and it needs an authenticated MCP user.

**Evidence:** by reading. The DDEV data set is too small to reproduce the memory limit.

**Fix**
- Use one query: `SELECT ID, post_status, post_type, post_author, post_title … LIKE …`. Derive `count`, the first 50 items and the live list in PHP, instead of COUNT + SELECT + `liveIds()`.
- Evaluate `edit_post` in chunks of about 200 IDs, calling `_prime_post_caches()` per chunk and `clean_post_cache()` or `wp_cache_flush_runtime()` between chunks.
- Or fail closed: above a hard cap (e.g. 1,000 live usages), refuse the confirmed edit with a hint ("im Divi-Builder bzw. von einem Administrator bearbeiten").

### L2 – Low (global edits): "global" is decided by the scope term, but Divi renders every referenced entry, so edits to referenced non-global entries skip the dry run and P8a

**Where**
- `src/Integrations/Divi/Library/GlobalEdits.php:71` and `:88-90`: `if (! $global) return LibraryItem`.
- `src/Integrations/Divi/Library/DiviLibraryStore.php:37` (`isGlobal()` → `GlobalLayout::is_global_layout_template()`, which checks the scope term, with a meta fallback for modules only).
- Divi: `BlockParser.php:838-880` renders any `divi/global-layout` whose `globalModule` names a *published `et_pb_layout`*. It checks only `post_type` and `post_status`, not the scope.

**Scenario**
1. A live page that user U cannot edit holds an instance of library entry N, but N has no `global` scope term. This can happen when:
   - the entry comes from an import (Divi's D4 fallback covers modules only, not sections or rows);
   - the scope was changed later;
   - the markup was written outside the Divi builder.
2. U has `edit_post` on N (as its author, or as an editor) and changes N with any Divi tool.
3. `GlobalEdits` treats N as a normal entry: no dry run, no `affected` list, no P8a check, no site-wide CSS clear.
4. The page U cannot edit now shows the changed content.

**Evidence (live)**
- Temp entry 1910 (`scope = non_global`) and temp published page 1911 with `<!-- wp:divi/global-layout {"globalModule":"1910","blockName":"divi/text"} /-->`.
- `isGlobal(1910) = false`, while `GlobalUsages::liveIds(1910) = [1911]`.
- The frontend HTTP render of 1911 shows the marker from 1910 (`LITAUDIT_NORMAL_ENTRY`).
- Both posts were deleted afterwards.

**Fix:** in `GlobalEdits::prepare()` and `notice()`, apply the global rules (confirm, `affected`, P8a, `GlobalItem` scope) whenever `isGlobal()` is true **or** `GlobalUsages` finds at least one usage. For the check, one `LIMIT 1` query is enough.

### L3 – Low (disclosure): the `lit/library-list` cursor is an oracle over the *content* of entries the caller may not read

**Where**
- `src/Integrations/Divi/Library/LibraryListAbility.php:176-194`: rows the caller may not read still advance `$position`, and `next_cursor = Cursor::encode($position)` is `base64("o:N")`.
- `src/Integrations/Divi/Library/DiviLibraryStore.php:103`: `'s' => search` makes WP search title, excerpt **and content**, although the schema at `LibraryListAbility.php:68` says "Suche im Titel".

**Scenario**
1. An author with the Divi library right creates two entries of their own that contain candidate words.
2. They then search with `limit: 1`.
3. The decoded cursor tells how many hidden entries match the search and sort before their own. That lets them guess words from other users' library content (codes, internal notes).
4. Plugin rule E3 / P12 is that authors may not read those entries: `lit/library-list id=…` returns `lit_forbidden`.
5. Mitigation: Divi's own Visual Builder library lists all entries to every user with the library right. Relative to Divi this is parity; relative to the plugin's own stricter rule it is a bypass. It is the same defect class that `PostGuard::ownPostsOnly()` already documents.

**Evidence (live)**
- Temp author and 3 temp entries: an admin entry "aaa" containing `LITAUDITSECRET pin 4711`, and author entries "zzz1" and "zzz2" containing `LITAUDITSECRET`.
- As the author, `search=LITAUDITSECRET, limit=1` returns items `["LITAUDIT zzz1"]` with `next_cursor=bzoy` (= `o:2`, which reveals one hidden match).
- A direct read of the admin entry returns `lit_forbidden`.
- All temp data was deleted and the deletion verified.

**Fix**
- Restrict the query in `find()`:
  - add `'author' => PostGuard::ownPostsOnly(DiviLibraryStore::POST_TYPE)` when it is non-null, so the cursor counts only rows the caller can see;
  - set `'search_columns' => ['post_title']` (WP ≥ 6.2) to match the documented title-only search.
- Keep the per-row `canRead()` filter as defence in depth.

### L4 – Low (disclosure): `affected`, `blocked_ids` and the P8a refusal expose IDs, status and type of posts the caller may not read

**Where**
- `src/Integrations/Divi/Library/GlobalUsages.php:48-56`: title masked, but `post_id`, `status` and `type` are always returned.
- `src/Integrations/Divi/Library/GlobalEdits.php:184-203`: IDs in message and details.
- Also `lit/library-list id=…` (`affected`).

**Scenario**
1. An author owns a global entry. An administrator used it on private pages and on other users' drafts.
2. The author's dry run lists those posts' IDs, their status (`private`, `draft`, `future`) and their post type.
3. Only the titles are hidden.

**Evidence:** by reading. The masking applies to `post_title` only.

**Fix**
- For rows without `read_post`, return only an aggregate: `hidden_count`, plus the count of blocking hidden posts.
- Name blocked IDs only where `read_post` holds; otherwise say "N Einträge, die dieses Konto nicht lesen darf".

### L5 – Low (authorization): `lit/divi-node-copy` from a library entry into a page skips Divi's role option `add_library`

**Where**
- `src/Integrations/Divi/Pages/DiviNodeCopyAbility.php:78-96`: the source goes through `DiviPageLoader::load(…, false)`, i.e. `LibraryAccess::item()` / `canRead()` (`edit_posts`, `divi_library`, `edit_post`).
- Compare `LibraryAccess::canInsert()` (`add_library`), which `lit/library-insert` requires.

**Scenario**
1. The site owner turns off "Aus der Bibliothek hinzufügen" (add from library) for a role in Divi → Rollen-Editor.
2. A user of that role can still copy any node from an entry they can edit into a page, via `lit/divi-node-copy` with `source_post_id = <library entry>`.
3. Security impact is small, because content still passes StoredNodeGuard. But it breaks E3 ("nie laxer als Divi", never laxer than Divi).

**Evidence:** by reading.

**Fix:** in `precheck()`, when the loaded source `isLibraryItem()`, additionally return `$access->canInsert()`. The target side is already covered by `canEdit()` / `save_library`.

### L6 – Low (global edits): P8a accepts Theme Builder layouts on WordPress `edit_post` alone

**Where**
- `src/Integrations/Divi/Library/GlobalEdits.php:158-171`.
- On DDEV, `et_header_layout`, `et_template`, `et_theme_builder` and `et_pb_layout` all use capability type `post` with `map_meta_cap`. So an editor has `edit_post` on the site header and footer layouts.
- Divi gates the Theme Builder with the role option `theme_builder`, which is off by default for every role but administrator.

**Scenario**
1. A global module sits in the Theme Builder header, footer or body.
2. An editor with `edit_global_library` passes P8a, because they have `edit_post` on the TB layout.
3. The confirmed edit changes the site-wide header, although Divi does not let this editor use the Theme Builder.
4. Divi's own Visual Builder allows the same global edit from any page (parity with Divi). But P8a's intent, "changes only on posts this account may edit", is not met.

**Evidence:** live, post type objects and caps (probe p1). The rest is by reading.

**Fix:** in `blocked()`, also treat IDs whose post type is one of `et_header_layout`, `et_body_layout`, `et_footer_layout` or `et_template` as blocked unless `DiviRoles::allows('theme_builder')`.

---

## Info / hardening

### I1 – StoredNodeGuard trusts the instance's own `blockName` and `globalModule`; Divi renders the real entry

**Where**
- `src/Integrations/Divi/Compose/StoredNodeGuard.php:90-91` and `:114-138`. Routing and the code-module check use `GlobalInstance::effectiveName()`, i.e. the stored `blockName`.
- `Json::decodeObject()`: the last duplicate key wins.

**What Divi does**
- `BlockParser.php:838` takes the *first textual* `"globalModule":"(\d+)"`. That can be a nested one inside `localAttrs`, or the first of two duplicate keys.
- `BlockParserStore.php:1113ff` merges `localAttrs` (filtered by the entry's `localAttrsMap`) onto the entry's *actual* root block.

**Consequence**
- A forged instance can be routed as `divi/text` (kses-checked richtext) while it is rendered onto a `divi/code` entry, or onto another entry than the guard saw.
- Exploiting this needs a forged instance in a source the user can edit. The user can already write that into their own page through core REST (parity, cf. 2b audit M1). The new part is that node-copy can carry it into a library entry (`et_pb_layout` has `show_in_rest = false`).

**Hardening**
- Refuse instances whose JSON contains more than one `"globalModule"` occurrence.
- Resolve `globalModule` to a published `et_pb_layout` the user may read.
- Require its root block name to equal `blockName`, and route `localAttrs` by that real name.

### I2 – Confirm is not bound to the dry run

- `GlobalEdits.php:95-117`: between the dry run and `confirm: true`, the `affected` set can grow; the user agreed to N pages, and M are changed.
- Rights are safe, because P8a is recomputed at confirm (verified by reading; a live check exists from T9).
- Optional: accept `expected_affected_count` (or a hash of the affected IDs) and refuse on mismatch.

### I3 – GlobalUsages misses non-canonical references Divi still renders

- `"globalModule":"0123"` renders entry 123: Divi's `([0-9]+)` capture is cast to int by `get_post()`.
- The LIKE on `"globalModule":"123"` does not match it.
- Only someone who can edit the containing post can write it, so there is no attacker gain.
- Optional: LIKE on `'"globalModule":"%'` plus a PHP regex with `ltrim($id, '0')`.

**Not a gap (checked live):** Divi 4 shortcode pages with `global_module="<id>"`. GlobalUsages does not find them (probe p2/p3: `liveIds = []`), but Divi 5 does not render the D5 entry content there either. The frontend showed the page's own local D4 content, not the entry's. The plugin refuses to edit D4-format entries, so no unlisted page changes.

---

## Areas checked and found sound

- **IDOR via `library_id`:** `LibraryInsertAbility::item()` goes through `LibraryAccess::item(false)` (P5/P12): same `not_found` for non-library IDs and revisions, `forbidden` only for users with the library right.
- **IDOR via `source_post_id`:** PostGuard (`edit_post`, managed types only; `revision`, TB and library types excluded) or LibraryAccess, re-checked in `execute()`, with its own `source_tree_rev`.
- **IDOR via `post_id` in `$variable` settings / `{{link:ID}}`:** `select_post` → `read_post` (`DynamicContentRules.php:246-259`), re-checked by StoredNodeGuard through `canonical()` against the target post. Only `post_link_url_*` uses `select_post` in Divi 5.2.1. `{{link:}}` to `et_pb_layout` is refused by the registry, with a uniform message.
- **Divi role options:**
  - `divi_library`, `save_library`, `add_library` and `edit_global_library` are combined correctly with `edit_posts`, `edit_post` and `publish_posts` in `LibraryAccess` (except L5).
  - The P9 group gates (writable/visible) are applied in `item()`, `canEdit()`, `canSave()` and `canInsert()`.
- **Global edits through replace-text, node-copy into, insert into, and delete in a global entry:**
  - All go through `DiviWriter::commit()` → `GlobalEdits` (shape, dry run, P8a, backup, `GlobalItem` scope).
  - replace-text never sends `confirm`.
  - An instance inside a global entry is refused (P11/`shapeViolation`).
  - Generic WP tools (`content-update`, `post-trash`, `revision-restore`, `duplicate`) cannot reach `et_pb_layout` (not managed).
- **Dry run/confirm race for rights:** P8a is recomputed at confirm from a fresh query. The `tree_rev` lock covers entry content changes.
- **Content injection:**
  - `localAttrs` and the snapshot children of instances are walked, and so are local children, because the parser keeps inner blocks of opaque blocks.
  - `deglobalize` runs before the guard.
  - CRLF unification touches whitespace-only text between blocks only.
  - Invalid or non-`divi/` `blockName` instances are refused as opaque.
  - Library titles go through `sanitize_text_field` (plus core title kses).
  - Terms are limited to existing names.
- **Divi frontend sinks beyond interactions and Lottie** (scan of all `script-library-*.js`):
  - module link → `diviElementLinkData`, where `LinkUtils::generate_data` applies `esc_url` before `window.location`/`window.open`;
  - multi-view `innerHTML`/`setAttribute`: data via `wp_localize_script` (nested values are not entity-decoded), with fixed attribute names and per-attribute sanitizers;
  - `new Function` hits are bundler global shims;
  - magnific-popup, slider and menu read `src`/`href` from escaped markup.
  - Together with the T11 live URL matrix, no new sink was found.
- **Disclosure:**
  - `dynamic-sources` samples only an editable post, uses the `edit` context (custom meta hidden without `read_dynamic_content_custom_fields`) and returns no option defaults.
  - Render-summary notes list only IDs already present in content the caller may read.
  - site-status `problems` are labels of missing Divi internals only.
  - Error texts are uniform for missing and unreadable IDs (P12; `{{link:}}` from T2). The known P13 leftovers are left to the final wave.
- **Multisite:** everything is per-blog (`$wpdb->posts`, Divi role options, `current_user_can`). No network-level path exists. Without `unfiltered_html` (site admins), the guard and the P9 backup refusal only get stricter.
- **DoS (other):**
  - `lit/library-list` scan is capped (`MAX_SCAN` 500, `limit` ≤ `Cursor::MAX_LIMIT`).
  - Guide `topic=all` is capped at 100 KB of static files.
  - `affected` is capped at 50 rows (the cost is in L1, not the output).