Sanitize early, escape late. #1
Conversation
|
Looking good from here, do you wan this mering? |
| @@ -188,7 +188,7 @@ public function update( $post_id, $post = '' ) { | |||
| $current_roles = members_get_post_roles( $post_id ); | |||
|
|
|||
| // Get the new roles. | |||
| $new_roles = isset( $_POST['members_access_role'] ) ? $_POST['members_access_role'] : ''; | |||
| $new_roles = isset( $_POST['members_access_role'] ) ? array_map( 'members_sanitize_role', $_POST['members_access_role'] ) : ''; | |||
|
Thanks @rmccue, have:
Happy for this to be merged in to our fork, I'll then submit a PR upstream for hardening. |
| @@ -204,7 +204,7 @@ public function update( $post_id, $post = '' ) { | |||
| $old_message = members_get_post_access_message( $post_id ); | |||
|
|
|||
| // Get the new message. | |||
| $new_message = isset( $_POST['members_access_error'] ) ? stripslashes( wp_filter_post_kses( addslashes( $_POST['members_access_error'] ) ) ) : ''; | |||
| $new_message = isset( $_POST['members_access_error'] ) ? stripslashes( wp_filter_post_kses( addslashes( wp_unslash( $_POST['members_access_error'] ) ) ) ) : ''; | |||
There was a problem hiding this comment.
Not being changed here, but relevant: this should use wp_kses_post instead of messing with slashing.
There was a problem hiding this comment.
@rmccue do you mean?
$new_message = isset( $_POST['members_access_error'] ) ? wp_kses_post( wp_unslash( $_POST['members_access_error'] ) ) : '';
or is wp_unslash unneeded too?
There was a problem hiding this comment.
The strip/add slashes here is just undoing what wp_filter_post_kses does internally, but it can use wp_kses_post to avoid that and remove the calls here. The wp_unslash call would still be needed for $_POST, so yes, the line that you have there would work.
|
@rmccue thanks, back to you for final approval |
Note:
members_sanitize_roleis a slightly stricter version ofsanitize_key, replacing hyphens with underscores.