Files
WP-LIT-MCP/docs/superpowers/notes/2026-09-27-etappe-3a-security-audit.md
2026-09-27 04:57:43 +02:00

16 KiB
Raw Permalink Blame History

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).