Skip to content

Commit adf51ac

Browse files
ilicfilipclaude
andauthored
v1.9.1 (#763)
* Sanitize and escape prpl_recommendations title An authenticated Editor (or higher) could create a recommendation via POST /wp/v2/prpl_recommendations with an HTML payload in the `title` field (e.g. `<img src=x onerror=alert(1)>`). The dashboard JS template (views/js-templates/suggested-task.html) renders `title.rendered` with Underscore's unescaped `{{{ }}}` syntax, so the payload executed when an admin loaded the dashboard. Defense in depth: - Input: add a `rest_pre_insert_prpl_recommendations` filter that strips tags from `post_title` on every REST insert/update, regardless of the user's `unfiltered_html` capability. Recommendation titles are plain text, so this neutralizes the payload at the source. - Output (JS): route the two raw `{{{ }}}` title sinks through a new `prplSuggestedTask.sanitizeTitle()` helper, which inert-parses the value with DOMParser (no script/resource side effects) and re-escapes it, preserving legitimate entities like `&amp;` without double-encoding the server-side `esc_html`'d provider titles. - Output (admin bar): the PRPL debug tool printed `post_title` unescaped into a `WP_Admin_Bar` node id (an HTML attribute) and title (rendered as raw HTML), firing the payload on every admin page in debug mode. Escape the title with `esc_html()`, use the post ID for the node id, and escape the activities node title too. - Also switch `updateTaskTitle` to set `.textContent` instead of `.innerHTML` for the screen-reader label, closing a self-XSS sink. Adds tests/phpunit/test-class-rest-recommendations-xss.php covering Editor and Administrator payloads plus a plain-text regression check. * Bump version to 1.9.1 * add migration script and revert JS title escaping * add inline comment, cc @tacoverdo * Delete recommendation when sanitized title is empty A title that is pure markup strips to an empty string. wp_update_post() rejects an update that would leave the title, content, and excerpt all empty, so the malicious title was left in the DB. The plugin never stores title-less recommendations, so delete such rows instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * update readme.txt --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d288841 commit adf51ac

9 files changed

Lines changed: 389 additions & 6 deletions

File tree

assets/js/suggested-task.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,11 +472,13 @@ prplSuggestedTask = {
472472
} )
473473
)
474474
);
475+
// Use textContent (not innerHTML) so a title typed into the
476+
// contenteditable field cannot inject markup.
475477
el
476478
.closest( 'li.prpl-suggested-task' )
477479
.querySelector(
478480
'label:has(.prpl-suggested-task-checkbox) .screen-reader-text'
479-
).innerHTML = `${ title }: ${ prplL10n( 'markAsComplete' ) }`;
481+
).textContent = `${ title }: ${ prplL10n( 'markAsComplete' ) }`;
480482
}, 300 );
481483
},
482484

classes/class-suggested-tasks.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ public function __construct() {
6767
// Filter the REST API response.
6868
\add_filter( 'rest_prepare_prpl_recommendations', [ $this, 'rest_prepare_recommendation' ], 10, 2 );
6969

70+
// Sanitize the recommendation title on insert/update via the REST API, to prevent stored XSS.
71+
\add_filter( 'rest_pre_insert_prpl_recommendations', [ $this, 'rest_sanitize_recommendation' ], 10, 2 );
72+
7073
\add_filter( 'wp_trash_post_days', [ $this, 'change_trashed_posts_lifetime' ], 10, 2 );
7174
}
7275

@@ -491,6 +494,28 @@ public function rest_api_tax_query( $args, $request ) {
491494
return $args;
492495
}
493496

497+
/**
498+
* Sanitize a recommendation before it is inserted or updated via the REST API.
499+
*
500+
* Recommendation titles are plain text (they are rendered unescaped in JS
501+
* templates such as views/js-templates/suggested-task.html), so we strip any
502+
* HTML tags here to prevent stored XSS. This runs regardless of the user's
503+
* `unfiltered_html` capability, which WordPress would otherwise honor for the
504+
* post title.
505+
*
506+
* @param \stdClass $prepared_post An object representing a single post prepared for inserting or updating the database.
507+
* @param \WP_REST_Request $request The request object.
508+
*
509+
* @return \stdClass The sanitized post object.
510+
*/
511+
public function rest_sanitize_recommendation( $prepared_post, $request ) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed
512+
if ( isset( $prepared_post->post_title ) ) {
513+
$prepared_post->post_title = \sanitize_text_field( \wp_strip_all_tags( $prepared_post->post_title ) );
514+
}
515+
516+
return $prepared_post;
517+
}
518+
494519
/**
495520
* Filter the REST API response.
496521
*
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
/**
3+
* Update class for version 1.9.1.
4+
*
5+
* @package Progress_Planner
6+
*/
7+
8+
namespace Progress_Planner\Update;
9+
10+
/**
11+
* Update class for version 1.9.1.
12+
*
13+
* @package Progress_Planner
14+
*/
15+
class Update_191 {
16+
17+
const VERSION = '1.9.1';
18+
19+
/**
20+
* Run the update.
21+
*
22+
* @return void
23+
*/
24+
public function run() {
25+
$this->sanitize_recommendation_titles();
26+
}
27+
28+
/**
29+
* Strip HTML tags from existing recommendation titles.
30+
*
31+
* Titles are now sanitized on write, but rows stored before that may still
32+
* contain markup. This cleans them up.
33+
*
34+
* @return void
35+
*/
36+
private function sanitize_recommendation_titles() {
37+
$db = \progress_planner()->get_suggested_tasks_db();
38+
39+
// get() defaults to all relevant statuses (publish, trash, draft, future, pending).
40+
$recommendations = $db->get();
41+
42+
foreach ( $recommendations as $recommendation ) {
43+
$sanitized = \sanitize_text_field( \wp_strip_all_tags( $recommendation->post_title ) );
44+
45+
// Nothing to do if stripping didn't change the title.
46+
if ( $sanitized === $recommendation->post_title ) {
47+
continue;
48+
}
49+
50+
// A title that was pure markup strips to an empty string. The plugin
51+
// never stores title-less recommendations (Suggested_Tasks_DB::add()
52+
// rejects them), so such a row is junk - delete it. This also avoids
53+
// WordPress's empty-content guard, which would otherwise reject an
54+
// update that leaves the title empty and leave the markup in place.
55+
if ( '' === $sanitized ) {
56+
$db->delete_recommendation( (int) $recommendation->ID );
57+
continue;
58+
}
59+
60+
$db->update_recommendation( $recommendation->ID, [ 'post_title' => $sanitized ] );
61+
}
62+
}
63+
}

classes/utils/class-debug-tools.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,11 @@ protected function add_suggested_tasks_submenu_item( $admin_bar ) {
273273

274274
$admin_bar->add_node(
275275
[
276-
'id' => 'prpl-suggested-' . $key . '-' . $title,
276+
// Use the post ID (not the title) for the node ID, and escape the
277+
// title for display - the admin bar renders 'title' as raw HTML.
278+
'id' => 'prpl-suggested-' . $key . '-' . $task->ID,
277279
'parent' => 'prpl-suggested-' . $key,
278-
'title' => $title . ' <a href="' . \esc_url( $delete_url ) . '" style="color: #dc3232; display: inline-block; margin-left: 5px; text-decoration: none;">×</a>',
280+
'title' => \esc_html( $title ) . ' <a href="' . \esc_url( $delete_url ) . '" style="color: #dc3232; display: inline-block; margin-left: 5px; text-decoration: none;">×</a>',
279281
]
280282
);
281283
}
@@ -307,7 +309,7 @@ protected function add_activities_submenu_item( $admin_bar ) {
307309
[
308310
'id' => 'prpl-activity-' . $activity->id,
309311
'parent' => 'prpl-activities',
310-
'title' => $activity->data_id . ' - ' . $activity->category,
312+
'title' => \esc_html( $activity->data_id . ' - ' . $activity->category ),
311313
]
312314
);
313315
}

progress-planner.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
* Description: A plugin to help you fight procrastination and get things done.
1010
* Requires at least: 6.6
1111
* Requires PHP: 7.4
12-
* Version: 1.9.0
12+
* Version: 1.9.1
1313
* Author: Team Emilia Projects
1414
* Author URI: https://prpl.fyi/about
1515
* License: GPL-3.0+

readme.txt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Tags: planning, maintenance, writing, blogging
44
Requires at least: 6.3
55
Tested up to: 6.8
66
Requires PHP: 7.4
7-
Stable tag: 1.9.0
7+
Stable tag: 1.9.1
88
License: GPL3+
99
License URI: https://www.gnu.org/licenses/gpl-3.0.en.html
1010

@@ -83,6 +83,11 @@ https://youtu.be/e1bmxZYyXFY
8383

8484
== Changelog ==
8585

86+
= 1.9.1 =
87+
88+
- Security fix: Authenticated (Editor and above) Stored Cross-Site Scripting (XSS) via recommendation titles. Titles are now sanitized when saved, and existing recommendations are cleaned up via an update script.
89+
- Thanks to [hongdo](https://patchstack.com/database/researchers/b19114df-00a1-4c42-b2f1-627b22001d57) for responsibly disclosing this issue via the Patchstack Bug Bounty Program.
90+
8691
= 1.9.0 =
8792

8893
In this release we've added an integration with the **All In One Seo** plugin so you’ll now see personalized suggestions based on your current SEO configuration.
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
<?php
2+
/**
3+
* Test that recommendation titles created via the REST API are sanitized.
4+
*
5+
* Regression test for the stored XSS reported by Patchstack: an Editor (or
6+
* higher) could POST /wp/v2/prpl_recommendations with a `title` containing an
7+
* HTML payload (e.g. `<img src=x onerror=alert(1)>`) which was later rendered
8+
* unescaped in the admin dashboard.
9+
*
10+
* @package Progress_Planner\Tests
11+
*/
12+
13+
namespace Progress_Planner\Tests;
14+
15+
/**
16+
* Class Rest_Recommendations_Xss_Test
17+
*/
18+
class Rest_Recommendations_Xss_Test extends \WP_UnitTestCase {
19+
20+
/**
21+
* Set up the REST server before each test.
22+
*
23+
* @return void
24+
*/
25+
public function setUp(): void {
26+
parent::setUp();
27+
28+
global $wp_rest_server;
29+
$wp_rest_server = new \WP_REST_Server();
30+
\do_action( 'rest_api_init' );
31+
}
32+
33+
/**
34+
* Tear down the REST server after each test.
35+
*
36+
* @return void
37+
*/
38+
public function tearDown(): void {
39+
global $wp_rest_server;
40+
$wp_rest_server = null;
41+
parent::tearDown();
42+
}
43+
44+
/**
45+
* Submit a recommendation via the REST API and return the raw response.
46+
*
47+
* @param string $title The title to submit.
48+
* @return \WP_REST_Response The REST response.
49+
*/
50+
private function submit_recommendation_via_rest( $title ) {
51+
$request = new \WP_REST_Request( 'POST', '/wp/v2/prpl_recommendations' );
52+
$request->set_header( 'Content-Type', 'application/json' );
53+
$request->set_body(
54+
(string) \wp_json_encode(
55+
[
56+
'title' => $title,
57+
'status' => 'publish',
58+
]
59+
)
60+
);
61+
62+
return \rest_get_server()->dispatch( $request );
63+
}
64+
65+
/**
66+
* Create a recommendation via the REST API and return the created WP_Post.
67+
*
68+
* @param string $title The title to submit.
69+
* @return \WP_Post The created post.
70+
*/
71+
private function create_recommendation_via_rest( $title ) {
72+
$response = $this->submit_recommendation_via_rest( $title );
73+
$this->assertSame( 201, $response->get_status(), 'Recommendation should be created.' );
74+
75+
$data = $response->get_data();
76+
return \get_post( $data['id'] );
77+
}
78+
79+
/**
80+
* An Editor submitting a title that is purely an HTML payload must not result
81+
* in a stored post that contains the markup.
82+
*
83+
* Stripping the tags leaves an empty title, so WordPress rejects the request
84+
* outright (the malicious post is never created) - an acceptable, even
85+
* preferable, outcome. We assert that either nothing was stored, or if it was,
86+
* the markup is gone.
87+
*
88+
* @return void
89+
*/
90+
public function test_editor_xss_title_is_stripped() {
91+
$editor_id = self::factory()->user->create( [ 'role' => 'editor' ] );
92+
\wp_set_current_user( $editor_id );
93+
94+
$response = $this->submit_recommendation_via_rest( '<img src=x onerror=alert(1)>' );
95+
96+
if ( 201 === $response->get_status() ) {
97+
$post = \get_post( $response->get_data()['id'] );
98+
$this->assertStringNotContainsString( '<img', $post->post_title );
99+
$this->assertStringNotContainsString( 'onerror', $post->post_title );
100+
$this->assertStringNotContainsString( '<', $post->post_title );
101+
} else {
102+
// An empty title (after stripping) is rejected; no post is created.
103+
$this->assertSame( 400, $response->get_status() );
104+
}
105+
}
106+
107+
/**
108+
* A <script> payload from an Editor should be stripped from the stored title.
109+
*
110+
* @return void
111+
*/
112+
public function test_editor_script_title_is_stripped() {
113+
$editor_id = self::factory()->user->create( [ 'role' => 'editor' ] );
114+
\wp_set_current_user( $editor_id );
115+
116+
$post = $this->create_recommendation_via_rest( '<script>alert(document.cookie)</script>Hello' );
117+
118+
$this->assertStringNotContainsString( '<script', $post->post_title );
119+
$this->assertStringNotContainsString( '</script', $post->post_title );
120+
// The plain-text remainder is preserved.
121+
$this->assertStringContainsString( 'Hello', $post->post_title );
122+
}
123+
124+
/**
125+
* Even an Administrator (who has the `unfiltered_html` capability) should
126+
* have HTML stripped from recommendation titles, since they are rendered as
127+
* plain text in JS templates.
128+
*
129+
* @return void
130+
*/
131+
public function test_admin_with_unfiltered_html_title_is_stripped() {
132+
$admin_id = self::factory()->user->create( [ 'role' => 'administrator' ] );
133+
\wp_set_current_user( $admin_id );
134+
135+
$post = $this->create_recommendation_via_rest( '<img src=x onerror=alert(1)>Title' );
136+
137+
$this->assertStringNotContainsString( '<img', $post->post_title );
138+
$this->assertStringNotContainsString( 'onerror', $post->post_title );
139+
$this->assertStringContainsString( 'Title', $post->post_title );
140+
}
141+
142+
/**
143+
* Legitimate plain-text titles (including ampersands) must be preserved.
144+
*
145+
* @return void
146+
*/
147+
public function test_plain_text_title_is_preserved() {
148+
$editor_id = self::factory()->user->create( [ 'role' => 'editor' ] );
149+
\wp_set_current_user( $editor_id );
150+
151+
$post = $this->create_recommendation_via_rest( 'Buy milk & eggs' );
152+
153+
$this->assertSame( 'Buy milk & eggs', $post->post_title );
154+
}
155+
}

0 commit comments

Comments
 (0)