Files
WP-LIT-MCP/docs/superpowers/notes/2026-09-26-etappe-2b-security-audit.md
T
2026-09-26 22:18:23 +02:00

21 KiB
Raw Blame History

WP-LIT-MCP: whole-plugin security audit (S42 step 2)

  • Scope: branch etappe-2, HEAD 314b3d0.
  • Environment: WordPress 7.1.2, Divi 5.2.1 on DDEV.
  • Method:
    • Read all of src/, plus the relevant parts of vendor/wp-media/mcp-oauth and vendor/wordpress/mcp-adapter.
    • Ran read-only DDEV wp eval-file probes.
    • Ran one live, self-cleaning run of the real lit/content-create ability as a temporary Author:
      • user 67 and post 861 were created, then deleted;
      • the deletion was verified afterwards: no lit_audit_* user and no probe post remain.
    • No code was changed. The option lit_mcp_allowed_roles was widened in-process only, through the filter, never in the database.

No Critical or High findings. The Divi value pipeline itself held. The two most relevant problems are ways around it:

  • the raw content field;
  • the tool-group switch.

Findings (most severe first)

M1 – Medium: raw content lets users without unfiltered_html write the "builder-only" Divi values (the S7/S26 gates don't apply)

Where

  • src/Integrations/WordPress/Content/PostWriter.php:28 (content → post_content) and :58.
  • ContentCreateAbility.php:127-133.
  • ContentUpdateAbility.php:124 refuses content only when the post already uses Divi. Nothing refuses content that introduces Divi markup.
  • Compare src/Integrations/Divi/Compose/ValueCleaner.php:57 (BUILDER_ONLY).

Scenario

  1. An Author, or an Editor on multisite or with DISALLOW_UNFILTERED_HTML, calls lit/content-create (or lit/content-update on a non-Divi post).
  2. The content is Divi 5 block markup that sets:
    • a module's css.desktop.value.freeForm (e.g. body{…});
    • module.decoration.attributes (custom HTML attributes).
  3. kses (filter_block_content) and Divi's own wp_insert_post_data security filters keep both. Only on* attributes are stripped, by Divi.
  4. The page is now a Divi layout. From then on every lit-divi-* tool works on it inside the kses window.

The same values are refused with "nur im Divi-Builder" (only in the Divi builder) when sent through lit-divi-insert/-update (rulings S7/S26).

A prompt-injected Claude session can therefore do CSS injection or attribute injection on the live site. This is exactly what S26 was meant to prevent.

Evidence

  • Live run of wp_get_ability('lit/content-create')->execute() as a temporary Author (unfiltered_html=false):
    • freeForm CSS stored: true;
    • data-probe stored: true;
    • rendered <div class="et_pb_text_0 … et_pb_module et_flex_module" data-probe="1">.
  • Read-only kses probe (kses6.php): onmouseover is removed by Divi and never rendered. No script execution found. Divi renders a javascript: linkUrl as href="".

Parity note The same user can store the same markup through core REST (/wp/v2/posts) or the code editor. So this is no privilege escalation beyond WordPress core. It is a bypass of the plugin's own, explicitly decided gate. The pipeline guarantee "every path from ability input to post_content goes through ValueCleaner/Composer/TreeEditor" does not hold for content.

Fix proposal

  • In PostWriter::prepare() (create and update), refuse a content that contains <!-- wp:divi/ (and [et_pb_), with a hint to use divi / the Divi tools.
  • Alternative: parse it with Parser and run every Divi block through Composer/ValueCleaner (heavier).
  • At minimum, do this for users without unfiltered_html.

M2 – Medium: "nur lesen" / disabled Divi group does not stop Divi writes (content-create divi)

Where

  • src/Integrations/WordPress/WordPressIntegration.php:83 passes $this->divi (the ContentComposer) unconditionally.
  • ContentCreateAbility.php:113-119, 145 write the Divi layout whenever Divi 5 is loaded.
  • ExposurePolicy::disabledByReadonly() and enabled() only look at the ability's own group ("wordpress").

Scenario

  1. The admin sets group Divi to "nur lesen", or disables lit-divi-insert, or disables the whole group.
  2. The tools tab, readme ("schaltet alle schreibenden Tools einer Gruppe … ab", switches off all writing tools of a group) and acceptance note §7 all promise that no Divi writes happen.
  3. Claude can still create complete Divi layouts via lit-content-create with divi: [...]. That goes through the same kses window.

Related cross-group writes that still work with Divi "nur lesen":

  • lit/post-duplicate, lit/post-replace-with-copy, lit/revision-restore on Divi pages. These use stored content, so the impact is lower.

Evidence: by reading. The wiring is unconditional; ExposurePolicy has no cross-group notion.

Fix proposal

  • Give ContentCreateAbility a callable diviAllowed() that is true only when ExposurePolicy::mode() of lit/divi-insert is not disabled. That mode already includes the Divi group's read-only switch.
  • If Divi writes are not allowed, answer divi with unsupported, and drop the divi property from the input schema.
  • Document in the Tools tab that duplicate/replace/restore copy stored Divi content regardless of the Divi group.

M3 – Medium: URL import SSRF – DNS rebinding TOCTOU, core-version-dependent blocklist, error oracle

Where

  • src/Integrations/Media/MediaImporter.php:55-62 (wp_http_validate_url pre-check).
  • :119-139 (wp_safe_remote_get + CURLOPT_IPRESOLVE_V4).
  • :142 / :339-346 (the raw WP_Error message is returned).

Scenario 1: DNS rebinding

  • A prompt-injected Claude (e.g. a web page Claude read says "import this image") calls lit-media-upload-from-url with a hostname.
  • wp_http_validate_url() resolves the host with gethostbyname() and sees a public IP. cURL then resolves again; with TTL 0 it gets 127.0.0.1 or another internal host.
  • Redirects are re-validated by core (WP_Http::validate_redirects, verified in 7.1.2), but each redirect has the same TOCTOU gap.
  • Impact:
    • blind GET requests to internal services on ports 80/443/8080;
    • internal images/PDFs (e.g. internal dashboards rendering PNG) get copied into the public media library.

Scenario 2: version-dependent blocklist

  • On DDEV (core 7.1.2), wp_http_validate_url() blocks 169.254/16, 100.64/10, TEST-NETs, multicast etc. Verified: ssrf1.php, all 17 internal probes return false.
  • The plugin declares "Requires at least: 6.9". Older cores only block 0/8, 10/8, 127/8, 172.16/12 and 192.168/16.
  • If 6.9 lacks the newer ranges, http://169.254.169.254/ (cloud metadata) is reachable. Not verified on 6.9.

Scenario 3: error oracle

  • Download fehlgeschlagen: <cURL/WP message> and HTTP-Status %d let the caller tell apart:
    • refused;
    • timeout;
    • 404;
    • "wrong file type".

Evidence: by reading, plus the DDEV validation probe.

Fix proposal

  • In the existing http_api_curl hook:
    • resolve the host once;
    • check the IP with the plugin's own full private/reserved list (filter_var(..., FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE), plus 100.64/10);
    • pin it with CURLOPT_RESOLVE (host:port:ip).
  • Set 'redirection' => 0 and follow at most 3 redirects manually, applying the same resolve, check and pin to each.
  • Return a generic "Download fehlgeschlagen" message and log the detail.

M4 – Medium (only when Pods field restrictions are used): Pods field access restrictions are ignored

Where

  • src/Integrations/Pods/PodsFieldSource.php:73-114 (read/write through pods()->field() and pods()->save()).
  • PostWriter.php:226 (META_SOURCES excludes pods, so there is no per-field capability check).
  • PostPresenter::isHiddenField() only hides the password type.

Scenario

  • Pods fields can be restricted per field:
    • admin_only;
    • restrict_role;
    • restrict_capability;
    • read_only;
    • hidden.
  • Pods enforces these only in its forms (PodsForm::permission → pods_permission).
  • Its PHP API does not: PodsAPI::save_pod_item defaults from = 'save', and 'save' === $params->from skips pods_permission (pods/classes/PodsAPI.php:4868, 4994).
  • An Editor or Author with edit_post on the entry can therefore read (lit-content-get, lit-content-query field filter) and write (lit-content-update/create) fields the site owner restricted to admins or made read-only.

Evidence: by reading the plugin and the Pods source in wp-content/plugins/pods. Pods is not active on DDEV, so there was no live test.

Fix proposal

  • In PodsFieldSource::definition():
    • mark fields READONLY when pods_has_permissions($field) && ! pods_permission($field) or when read_only is set;
    • hide them (as isHiddenField does) when hidden is set or permission fails without read_only_restricted.
  • Re-check in write() before save().

L1 – Low: any logged-in user can complete OAuth and get a connection (application password + tokens)

Where

  • The vendor flow (mcp-oauth AuthorizeEndpoint/ConsentEndpoint: only is_user_logged_in()).
  • The plugin adds no gate: Plugin.php:110 boots it as-is.
  • The lit_mcp_use gate only applies at tool execution (AbilityRegistrar.php:56-63) and tool listing (VisibilityFilter).

Scenario

  • On sites with customer or subscriber accounts (WooCommerce, memberships), any customer can connect Claude.
  • mcp-oauth then creates an application password for them, even where core would not offer app passwords to that user.
  • It also issues access and refresh tokens, and mcp-adapter sessions in user meta.
  • The customers can't run tools. But the connections appear in "Zugriff", and the site accumulates credentials and sessions for users who were never meant to have them.

Evidence: by reading.

Fix proposal: before mcp-oauth's authorize/consent handler, refuse with a clear message when ! current_user_can('lit_mcp_use'). A hook such as template_redirect on its rewrite query var would work, and so would a filter if mcp-oauth offers one.

L2 – Low: release/build hygiene – dev vendor, tests namespace, direct file access

Where

  • vendor/ (dev install).
  • composer.json autoload-dev.
  • All src/**/*.php (no ABSPATH guard).
  • tests/Integration/divi-convert-fixture.php (the only integration script without a WP_CLI guard).
  • .gitattributes (only relevant if the zip is made with git archive, and vendor/ is git-ignored).

Scenario and evidence

  1. Dev vendor loaded at runtime. The current tree's Jetpack autoloader loads these files on every WP request:

    • mockery/library/helpers.php;
    • Mockery.php;
    • brain/monkey/inc/api.php;
    • phpunit/.../Assert/Functions.php.

    Source: vendor/composer/jetpack_autoload_filemap.php. No release script exists (spec §U is still open).

  2. Tests namespace survives --no-dev. Reproduced: composer install --no-dev in a temp dir still gives LIT\MCP\Tests\ → tests/ in vendor/composer/jetpack_autoload_psr4.php:21. The Jetpack autoloader keeps root autoload-dev. If tests/ ships, test classes are autoloadable in production.

  3. Direct HTTP access leaks paths and versions.

    • curl https://…/wp-content/plugins/WP-LIT-MCP/src/Admin/AccessTab.php returns a PHP fatal error with the full server path (display_errors on in DDEV).
    • …/tests/Integration/divi-convert-fixture.php → HTTP 200.
    • …/vendor/composer/installed.json → HTTP 200 (full dependency and version list).

Fix proposal

  • Add a build script:
    • composer install --no-dev -o;
    • copy only wp-lit-mcp.php, uninstall.php, readme.txt, src/, assets/, docs/guides/, vendor/;
    • no tests/, docs/superpowers, .superpowers, composer.*, phpunit/phpcs configs.
  • Add a CI check that the zip contains no vendor/phpunit, vendor/mockery or tests/.
  • Add defined('ABSPATH') || exit; to the src files (Plugin Check flags its absence), and the WP_CLI guard to divi-convert-fixture.php.

L3 – Low: tree_rev / rev lock is check-then-write (lost update)

Where

  • DiviWriter.php:116 (check) → :161 (save).
  • ContentUpdateAbility.php:115 → :156.
  • MediaUpdateAbility modified.

Scenario: two parallel writes on the same page (two Claude sessions, or Claude plus the Visual Builder) both pass the rev check. The later wp_update_post() silently overwrites the earlier change. The restore-point revision limits the damage.

Evidence: by reading. There is no lock or conditional update.

Fix proposal:

  • Take a short per-post lock around check and save (wp_cache_add/add_option('lit_mcp_lock_<id>') with expiry, or MySQL GET_LOCK).
  • Re-read post_content inside the lock and compare Rev again right before wp_update_post().

L4 – Low: deactivation and uninstall leave OAuth sessions valid

Where

  • Lifecycle.php:93-101.
  • Uninstaller.php:28-61. This is documented as intentional.

Scenario

  • Deactivating or deleting the plugin keeps these:
    • the mcp-oauth application passwords;
    • mcp_refresh_jti_* meta;
    • mcp_jwt_secret.
  • Reactivating it within 30 days revives every refresh token. The admin may believe that removing the plugin cut Claude off.
  • Plugin-owned post meta _lit_mcp_copy_of / _lit_mcp_backup_of also remains.

Evidence: by reading.

Fix proposal:

  • On uninstall, delete the connections ConnectionRepository identifies (marker meta plus https app_id).
  • Either delete _lit_mcp_* meta, or mention in readme and deactivation notice that connections survive.
  • Offer "alle Verbindungen widerrufen" (revoke all connections) in the Access tab.

I1 – Info: mcp-oauth logs MCP request bodies when WP_DEBUG and WP_DEBUG_LOG are on

Where: vendor/wp-media/mcp-oauth/inc/Transport/OAuthHttpTransport.php:128-137 (McpLogger::safe_request_body(), 8 KB).

Tool arguments (page content, private drafts, Base64 uploads) go to wp-content/debug.log, which is often web-reachable on customer staging sites.

Fix proposal: mention this in the readme, or turn it off (e.g. filter or constant, if mcp-oauth offers one; otherwise ask upstream).

I2 – Info: non-Divi writes re-kses Divi pages for users without unfiltered_html

These tools save through plain wp_update_post/wp_insert_post, so content_save_pre kses re-serializes the whole Divi layout:

  • lit/content-update (title/excerpt/fields);
  • publish/unpublish/schedule;
  • revision-restore;
  • post-duplicate;
  • post-replace-with-copy.

The byte-identity promise of spec §1.3/§5.2 does not hold for these tools. There is no security impact: it only ever sanitizes more.

Evidence: read-only probe kses2.php. All 7 stored Divi pages changed under editor-kses (e.g. an empty block becomes /-->).

I3 – Info: smaller observations

  1. Multisite super admin bypasses the role gate. A super admin always has lit_mcp_use (WP_User::has_cap), even if "Zugriff" allows no role. AccessTab::currentUserLosesAccess knows this; the UI text doesn't say it.
  2. Media usage lists unreadable posts. MediaUsageFinder.php:455-461 returns post ID and post type (not the title) for posts the user can't read. This lets a user learn that private posts exist. It is intentional, to block deletes.
  3. All post types with a UI are exposed. Every show_ui post type, including non-public ones (on DDEV: path), is exposed; only core, Divi internals and lit_mcp_excluded_post_types are excluded. Consider documenting the filter for data-protection purposes (form submissions, personal data as CPTs).
  4. Nested REST requests can block later calls. A rest_do_request() to another route during a tool call makes RequestContext false for the rest of that HTTP request (RequestContext.php:281). This fails closed, so later JSON-RPC calls in the same batch get forbidden. Functional only.
  5. confirm: true is not a human confirmation. Claude sets it itself; under prompt injection it is no safeguard. Keep the destructive tools indirect by default.

Checked and found sound

  • RequestContext: recomputed on every matched REST request, cleared after our route, and required by every ability. WP-CLI/STDIO and other routes fail closed.
  • Default mcp-adapter server: disabled; the only MCP route is /mcp/mcp-oauth-server (verified via rest_get_server()->get_routes()).
  • Core Abilities REST: our abilities have show_in_rest=false (only the 3 core/* abilities are in REST).
  • Transport and JWT: OAuthHttpTransport requires a Bearer JWT (HS256 pinned, exp, aud = our route, iss = home_url(), app password must still exist). The current user is set only from the JWT; cookie or Basic auth can't reach the tools.
  • Revocation:
    • AccessTab revoke deletes the app password, which the next request's check honours;
    • refresh-token rotation and reuse detection live in mcp-oauth;
    • revoke is limited to edit_user-able users and, on multisite, to site members.
  • ConnectionUsage: records only after a <400 response on our route, and only for sub === current user.
  • lit_mcp_use gate: dynamic cap via user_has_cap (no role DB changes); the option is sanitized to existing roles; the filter output is type-checked; the meta-tools' capability filters are set to lit_mcp_use.
  • Exposure:
    • disabled and "nur lesen" abilities are never registered, so execute-ability answers "not found";
    • DIRECT_ONLY abilities are mcp.public=false, so execute-ability refuses them;
    • lit/guide is always direct.
  • Annotations: every tool annotated readonly really doesn't write (render-summary runs the_content side effects only).
  • Per-object checks:
    • PostGuard::editable (edit_post, managed type, trash rules) on every content, status, revision, render and Divi ability, in both permission and execute;
    • revisions are checked via the parent and post_type === 'revision';
    • attachments via MediaGuard (edit_post/delete_post, type check);
    • menu items via item ID membership in the menu.
  • Nested IDs:
    • featured image, parent and relation fields: read_post plus type checks;
    • menu object_id: read_post, no private, unpublished only with a flag, taxonomy must be viewable;
    • Divi media/gallery and select_post: read_post;
    • terms: assign_terms/edit_terms;
    • meta: edit_post_meta/add_post_meta per key.
  • Query and search:
    • content-query/media-search/replace-text limit the query to the user's own posts without edit_others_posts and gate statuses;
    • hidden fields can't be used as a query filter.
  • Settings:
    • manage_options;
    • strict allow-list;
    • admin_email/users_can_register/default_role/siteurl/home explicitly blocked;
    • per-key validation (installed languages, valid timezones, published pages, image attachment).
  • Meta writes:
    • update_post_meta double-serializes a serialized string, so there is no object injection;
    • protected keys are refused through the meta caps;
    • clone and replace copy raw stored meta via $wpdb (no unserialize).
  • Menu URLs: only http/https/mailto/tel-style schemes, /path (not // or /\) or #; control characters and spaces are refused; esc_url_raw is applied.
  • Media files:
    • MIME allow-list intersected with the core-allowed types for the user, checked against the real content (wp_check_filetype_and_ext);
    • no SVG by default;
    • sanitize_file_name (no traversal, double extensions defused);
    • streamed size cap;
    • Base64 size checked before decoding;
    • temp files removed on every path;
    • download and preview only read files realpath-inside uploads;
    • caption and description go through wp_kses_post.
  • Error handling:
    • Throwables are mapped to internal_error, details only go to the PHP log;
    • the sideload error is hidden;
    • no SQL or path text reaches the client (apart from the M3 download messages).
  • SQL: every query is $wpdb->prepare()d. The only interpolations are table names and the constant status list; the log filters are whitelisted.
  • Change log: holds titles and references only, no content. Admin list output is escaped; revision links are gated by edit_post on the parent.
  • Admin UI:
    • every admin-post action has manage_options plus check_admin_referer (revoke nonce per UUID, clear-log);
    • settings go through options.php with settings_fields nonces and sanitize callbacks;
    • closedpostboxes is core AJAX with its nonce;
    • notices and tabs come from fixed maps;
    • everything is escaped (esc_js+esc_attr for confirm handlers).
  • Site Health and admin notices: static text only, escaped; the loopback check is cached and only runs on demand.
  • Guides: topic slug regex plus the glob'd file list (no traversal); filtered guides are site-owned.
  • Divi pipeline integration:
    • the kses window covers only RevisionPoint::capture plus the one wp_update_post and is restored in finally, only when kses was active;
    • Divi's own wp_insert_post_data security filters stay (verified: they strip on* attributes);
    • the insert, update, move, delete and duplicate-node tools only write through Composer/TreeEditor;
    • duplicate-node adds only the stored label plus a constant;
    • replace-text re-checks targets per post;
    • DynamicContentMap drops loop_* options and types come from the registry; the fixed rules (enable_html, meta keys, markers) remain.
  • Uninstall and deactivation: both remove their own options, transient, cron and table on every site of a network, never posts.
  • Limits and DoS:
    • cursor offset capped at 9 digits, page size ≤100;
    • batch caps (media-update 50, divi-update 50, insert/delete 20, replace-text 20 targets / dry run ≤100 pages, content-create divi 30 sections);
    • render summary strings bounded;
    • text search limited to 200 characters.