fix(security): dynamic-content meta disclosure, test-email relay, SVG upload sanitization#2908
Merged
Conversation
The dynamic-content REST endpoints authorize any published post, so postMeta/authorMeta/loggedInUserMeta getters were reachable with an attacker-controlled metaKey. `authorMeta` + `user_pass` returned the author's password hash unauthenticated; postMeta exposed protected `_`-prefixed keys (IDOR); loggedInUserMeta exposed user secrets. Add a shared is_protected_meta_key() guard (underscore-prefixed keys, user_pass, user_activation_key, session_tokens) and apply it in all three getters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
send_test_email() used the payload `to` address unsanitized in both wp_mail() and the From header, letting any edit_posts user relay mail to arbitrary addresses and smuggle header fragments. Wrap it in sanitize_email(), matching the sibling send_default_email(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Bundle Size Diff
|
Contributor
|
Plugin build for 553f97b is ready 🛎️!
|
2ed0b40 to
0574759
Compare
save_file_from_field() decided whether to run the SVG sanitizer from the client-supplied metadata name, so an SVG uploaded under a lying non-.svg name skipped sanitization. Base the decision on the actual file being saved ($file_data name/type), which is what wp_handle_sideload stores it under. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The unauthenticated /otter/v1/dynamic endpoint accepted any `fallback` image path under WP_CONTENT_DIR and readfile()'d it, letting a caller read arbitrary images anywhere in wp-content (bypassing URL-level media access control). Extract the path check into get_safe_fallback_path() and narrow the allowed base from wp-content to the uploads directory, which is where library-chosen fallbacks always live. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0574759 to
a42d5a6
Compare
Contributor
E2E TestsPlaywright Test Status: See serial and parallel matrix jobs Performance ResultsserverResponse: {"q25":398.5,"q50":402.85,"q75":441.4,"cnt":10}, firstPaint: {"q25":1383.5,"q50":1406.1,"q75":1469.7,"cnt":10}, domContentLoaded: {"q25":3697,"q50":3736.1,"q75":3762.3,"cnt":10}, loaded: {"q25":3698.8,"q50":3738.35,"q75":3764.6,"cnt":10}, firstContentfulPaint: {"q25":4191.8,"q50":4235.05,"q75":4260,"cnt":10}, firstBlock: {"q25":14892.5,"q50":14944.2,"q75":15046.8,"cnt":10}, type: {"q25":27.8,"q50":29.08,"q75":30.22,"cnt":10}, typeWithoutInspector: {"q25":26.36,"q50":27.16,"q75":28.93,"cnt":10}, typeWithTopToolbar: {"q25":34.93,"q50":35.8,"q75":36.78,"cnt":10}, typeContainer: {"q25":18.62,"q50":20.22,"q75":21.48,"cnt":10}, focus: {"q25":137.96,"q50":146.92,"q75":149.42,"cnt":10}, inserterOpen: {"q25":46.21,"q50":48.89,"q75":51,"cnt":10}, inserterSearch: {"q25":18.08,"q50":19.33,"q75":20.45,"cnt":10}, inserterHover: {"q25":6.14,"q50":6.54,"q75":7.28,"cnt":20}, loadPatterns: {"q25":1806.04,"q50":1840.34,"q75":1871.93,"cnt":10}, listViewOpen: {"q25":250.58,"q50":254.9,"q75":265.51,"cnt":10} |
get_client_ip() returned forwarded-header values verbatim, and the result is concatenated into the outbound iphub geolocation URL. Take the first entry of a comma-separated forwarded list and validate it with FILTER_VALIDATE_IP, returning '' when it is not a valid IP. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parse_query() applied the public o_post_type query var to every query via the global query var, letting a visitor override post_type on unrelated/secondary queries. Gate it to the main search query and read the var from the query being parsed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
validate_source_and_get_name() fetched the source URL with sslverify => false. Remove the flag so wp_remote_get verifies the certificate (default). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The review comparison block echoed title, permalink, price, discounted, description and feature titles pulled from a referenced review block's attributes without escaping (line 116 above already escaped, these were missed). Escape each at the sink (esc_html/esc_url/wp_kses_post). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The wordpress.org API author field was echoed unescaped while every sibling field was escaped. Run it through wp_kses_post (preserves the intended anchor markup, strips scripts/handlers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Stripe field block echoed the id/mappedName/fieldOptionName block attributes and the Stripe product image/description/name into HTML with no escaping. Escape the product sinks (esc_url/esc_attr/esc_html) and extract the field attributes into get_field_attributes() with esc_attr. Adds Test_Render_Escaping covering the review-comparison, plugin-card and Stripe field escaping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
abaicus
approved these changes
Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three security fixes surfaced by a scan of the plugin's REST/AJAX/upload surface. Each is a separate atomic commit with a regression test (test-first).
1. 🔴 Unauthenticated meta disclosure via dynamic content (
class-dynamic-content.php)The
/otter/v1/dynamic+/dynamic/previewendpoints authorize any published post (no auth, no nonce). Combined with an attacker-controlledmetaKey, this exposed:type=authorMeta&metaKey=user_pass→ the author's password hash (unauthenticated).type=postMeta&metaKey=<key>→ any protected_-prefixed post meta (IDOR).type=loggedInUserMeta→ arbitrary user secrets (e.g.session_tokens).Fix: a shared
is_protected_meta_key()guard (underscore-prefixed keys,user_pass,user_activation_key,session_tokens) applied in the three getters. Fixed in the getters rather thancheck_permission, since that callback is shared with the front-end dynamic-image endpoint and tightening it would break logged-out visitors.2. 🟡 Test-email recipient relay / header injection (
class-form-server.php)send_test_email()used the payloadtounsanitized inwp_mail()and theFrom:header. Wrapped insanitize_email(), matching the siblingsend_default_email().3. 🟡 SVG form upload sanitized by wrong field (
class-form-utils.php)save_file_from_field()chose whether to sanitize based on the client-suppliedmetadata['name'], so an SVG uploaded under a lying non-.svgname skipped sanitization (exploitable where Main's global prefilter is absent, e.g. VIP). Now decides on the actual file being saved.Testing
Added regression tests (
test-dynamic-content.php+5,test-form-server.php+1,test-svg-upload.php+1). All 135 tests across the three affected classes pass.🤖 Generated with Claude Code