Skip to content

fix: validate Content ID charset to block API path/query injection#575

Merged
galbus merged 2 commits into
mainfrom
claude/eloquent-kapitsa-201d53
Jul 19, 2026
Merged

fix: validate Content ID charset to block API path/query injection#575
galbus merged 2 commits into
mainfrom
claude/eloquent-kapitsa-201d53

Conversation

@galbus

@galbus galbus commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes an authenticated API path/query injection via an unvalidated Content ID. A WordPress user with edit_post (author, or contributor on their own draft) could store a crafted beyondwords_content_id and steer authenticated, organization-API-key-scoped requests to arbitrary BeyondWords API endpoints.

The vulnerability

beyondwords_content_id was persisted with only sanitize_text_field(), which permits /, ., ?, # and &. That value was later read back verbatim and interpolated unencoded into API URL paths:

sprintf( '%s/projects/%d/content/%s', $api_url, $project_id, $content_id )

in Client::get_content(), update_audio() and delete_audio(). Because Client::filter_http_request_args() attaches the org X-Api-Key header to any outbound URL that starts with the API base — and path-suffix manipulation preserves that prefix — a value such as:

x/../../projects/999/content/abc?force=1

resolves (after cURL dot-segment normalization) to an attacker-chosen endpoint under the API host, with the org key attached, letting an author-level user read/update/delete content in other projects of the organization. Both write paths were affected: the classic-editor metabox (ContentId::save()) and the block-editor/REST meta write (Sync::register_meta(), sanitize_callback => sanitize_text_field). The plugin's own inspect REST route already documented the intended charset as [a-zA-Z0-9\-]+.

What changed

Defense at three layers (per the advisory):

  1. New shared validatorMeta::sanitize_content_id(): sanitize_text_field() + a strict ^[a-zA-Z0-9-]*$ gate (the same charset the inspect REST route enforces), blanking anything else.
  2. Both write paths routed through it — ContentId::save() (classic editor) and the beyondwords_content_id sanitize_callback in Sync::register_meta() (block editor / REST). Only that one meta key gets the strict callback; others keep sanitize_text_field.
  3. Defense-in-depth at the sinkrawurlencode( (string) $content_id ) at the three Client URL builders. Transparent for legitimate IDs (numeric / UUID), so no behavior change for valid callers. (batch_delete already sends IDs in a JSON body, not a URL path — unchanged.)

Follows WordPress-VIP standards — input is unslashed + sanitized, and no phpcs:ignore was added.

Reviewer notes

  • The register_meta sanitize_callback applies globally, so the strict gate takes effect on every beyondwords_content_id write (including update_post_meta), not just REST. API-issued IDs (UUIDs/numeric) pass through unchanged.
  • Three pre-existing tests needed small adjustments for the now-stricter meta contract, documented inline:
    • The stored-XSS renderer test now raw-writes its payload to the DB (bypassing the sanitizer) so it still exercises the renderer's output-escaping — that second line of defense is preserved.
    • Two placeholder Content IDs containing underscores ('beyondwords_content_id', 'test_value') were changed to charset-valid values.

Testing

  • phpcs (full WordPress-VIP-Go scope): clean (exit 0).
  • Full PHPUnit suite: OK — 662 tests, 2079 assertions, 0 failures. Ran the whole suite (not just changed files) because the register_meta callback registers globally.
  • New tests: sanitize_content_id unit provider; ContentId::save injection cases; a Sync test proving update_post_meta blanks a crafted ID end-to-end; a Client test asserting the URL is %2F/%3F-encoded.
  • Per project guidance, the full Cypress suite was not run.

🤖 Generated with Claude Code

An unvalidated Content ID saved via the editor metabox or the
REST/block-editor meta write was interpolated raw into BeyondWords
API URL paths (sprintf '%s/projects/%d/content/%s'). The org
X-Api-Key header is attached to any URL under the API base, so a
crafted value such as 'x/../../projects/999/content/abc?force=1'
let an author-level user steer authenticated, org-scoped
GET/PUT/DELETE requests to arbitrary endpoints.

Fix in three layers:
- Meta::sanitize_content_id(): sanitize_text_field + a strict
  ^[a-zA-Z0-9-]*$ gate (the charset the inspect REST route uses),
  blanking anything else.
- Route ContentId::save() and the beyondwords_content_id
  register_meta sanitize_callback through it (both write paths).
- Defensively rawurlencode() the Content ID at the three Client
  URL builders (get_content, update_audio, delete_audio).

Verified: phpcs clean; full PHPUnit suite green (662 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.


🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

Comment thread src/post/class-meta.php
@galbus
galbus marked this pull request as ready for review July 19, 2026 07:02
Resolves a conflict in Client::delete_audio_by_ids(): main shortened
the DELETE timeout while this branch added rawurlencode() to the
Content ID. Kept both — the URL is encoded and the request passes
DELETE_TIMEOUT.

Also adapts main's new player_embed_neutralises_content_id_xss test.
Content IDs are now charset-validated on save, so update_post_meta()
blanked its hostile payload and no player rendered. It now writes the
payload straight to the DB, which preserves the classic-editor
escaping coverage and additionally fixes the exact-encoding assertion
that was already failing on main (sanitize_text_field had been
mangling the payload before it reached the renderer).

Verified: phpcs clean; full PHPUnit suite green (676 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@galbus
galbus merged commit 9a5631f into main Jul 19, 2026
9 checks passed
@galbus
galbus deleted the claude/eloquent-kapitsa-201d53 branch July 19, 2026 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants