-
Notifications
You must be signed in to change notification settings - Fork 151
Security: Fix capability checks & shortcode exec vuln #1711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ea20022
95e08fb
8cd0596
27ed3e4
b0e6aec
efcafde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,7 +152,7 @@ public function get_field_props() { | |
| public function prepare_entry( $field ) { | ||
| check_ajax_referer( 'wpuf_form_add' ); | ||
|
|
||
| $value = isset( $_POST[ $field['name'] ] ) ? sanitize_text_field( wp_unslash( $_POST[ $field['name'] ] ) ) : ''; | ||
| $value = isset( $_POST[ $field['name'] ] ) ? strip_shortcodes( sanitize_text_field( wp_unslash( $_POST[ $field['name'] ] ) ) ) : ''; | ||
| // return sanitize_text_field( trim( $_POST[$field['name']] ) ); | ||
| return $value; | ||
| } | ||
|
Comment on lines
152
to
158
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainSecurity fix correctly implements shortcode stripping. The addition of
Combined with the nonce check on line 153, this provides defense-in-depth against shortcode injection attacks. Run the following script to verify this pattern has been consistently applied across all field types mentioned in the AI summary: 🏁 Script executed: #!/bin/bash
# Description: Verify strip_shortcodes is consistently applied in prepare_entry methods
# Search for prepare_entry methods that handle POST data
rg -n -A 5 'function prepare_entry' --type=php -g 'includes/Fields/*.php' | \
rg -C 3 '\$_POST'Length of output: 8468 Add missing shortcode stripping in taxonomy field sanitization
$value = isset($_POST[$field['name']])
? strip_shortcodes(sanitize_text_field(wp_unslash($_POST[$field['name']])))
: '';🤖 Prompt for AI Agents |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,14 @@ public function __construct() { | |
| if ( ! isset( $_GET['wpuf_preview'] ) && empty( $_GET['wpuf'] ) ) { | ||
| return; | ||
| } | ||
| $this->form_id = isset( $_GET['form_id'] ) ? intval( $_GET['form_id'] ) : 0; | ||
|
|
||
| // Security: Check user has proper capabilities before allowing preview | ||
| if ( ! is_user_logged_in() || ! current_user_can( wpuf_admin_role() ) ) { | ||
| wp_die( __( 'You do not have permission to preview this form.', 'wp-user-frontend' ), 403 ); | ||
| } | ||
|
|
||
| // Security: Validate and sanitize form_id parameter | ||
| $this->form_id = isset( $_GET['form_id'] ) ? absint( $_GET['form_id'] ) : 0; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. recheck nonce verification |
||
| add_action( 'pre_get_posts', [ $this, 'pre_get_posts' ] ); | ||
| // add_filter( 'template_include', [ $this, 'template_include' ] ); | ||
| add_filter( 'the_title', [ $this, 'the_title' ] ); | ||
|
|
@@ -77,18 +84,18 @@ public function the_title( $title ) { | |
| * @return string | ||
| */ | ||
| public function the_content( $content ) { | ||
| if ( $this->is_preview ) { | ||
| if ( ! is_user_logged_in() ) { | ||
| return __( 'You must be logged in to preview this form.', 'wp-user-frontend' ); | ||
| } | ||
| $viewing_capability = apply_filters( 'wpuf_preview_form_cap', | ||
| 'edit_posts' ); // at least has to be contributor | ||
| if ( ! current_user_can( $viewing_capability ) ) { | ||
| return __( 'Sorry, you are not eligible to preview this form.', 'wp-user-frontend' ); | ||
| } | ||
| // Security: Double-check admin capabilities | ||
| if ( ! current_user_can( wpuf_admin_role() ) ) { | ||
| return __( 'You do not have permission to preview this form.', 'wp-user-frontend' ); | ||
| } | ||
|
|
||
| // Security: Validate form_id is a valid integer to prevent injection | ||
| $form_id = absint( $this->form_id ); | ||
| if ( $form_id === 0 ) { | ||
| return __( 'Invalid form ID.', 'wp-user-frontend' ); | ||
| } | ||
|
|
||
| return do_shortcode( sprintf( '[wpuf_form id="%d"]', $this->form_id ) ); | ||
| return do_shortcode( sprintf( '[wpuf_form id="%d"]', $form_id ) ); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect sanitization breaks rich text editing.
The use of
sanitize_text_field()strips all HTML tags and line breaks, thenwp_kses_post()becomes redundant. This breaks the rich text editing functionality supported by this field (see render method lines 41-87 wherewp_editoris used for rich/teeny modes). WordPress post excerpts typically allow basic HTML formatting.Apply this diff to fix the sanitization order while preserving the shortcode stripping security fix:
This approach:
wp_unslash()wp_kses_post()to allow post-content tagswp_kses_post()call📝 Committable suggestion
🤖 Prompt for AI Agents