lifterlms icon indicating copy to clipboard operation
lifterlms copied to clipboard

<iframe> and <script> tags should not be removed from the post content of certificates

Open thomasplevy opened this issue 3 years ago • 0 comments

Reproduction Steps

As a user with the unfiltered_html capability (an administrator)

Create a new certificate with either an <iframe> (say a video embed) or a <script> tag, for example: <script>alert( 'HELLO' );</script>

Expected Behavior

These tags would not be allowed in the content of the certificate because certificates are meant to be printed and scripts and (most) iframe content cannot be printed

Actual Behavior

These tags can be saved in the post content so long as the user creating the certificate has the unfiltered_html capability (administrators, for example)

Error Messages / Logs

N/A

This issue has be recreated:

  • [x] Locally
  • [ ] On a staging site
  • [x] On a production website
  • [x] With only LifterLMS and a default theme

Remediation

I recommend initializing the wp_kses() content filtering applied to post content only for the certificate post type for users with the unfilitered_html capability, something like:

add_filter( 'content_save_pre, function( $content ) {
   if ( current_user_can( 'unfiltered_html' ) {
      $content = wp_filter_post_kses( $content );
   }
   return $content;
} );

I think something link that will mimic the functionality for users without the capability as seen at : https://core.trac.wordpress.org/browser/tags/5.6/src/wp-includes/kses.php#L2093

Additional considerations

I don't foresee anyone ever being upset about not being able to put a script in their certificate but I can imagine a user potentially being unhappy about iframes... For example, what if someone is embedding a PDF in an iframe in their certificates... Or something like this? Is anyone doing this? Not that I'm aware but would someone be mad that they couldn't?

As such, we need to make sure that the filter we apply to prevent this can be easily removed with a remove_action() call so that if someone needs to drop frames or scripts (or something else stripped by the kses() function we can easily re-allow it.

thomasplevy avatar Feb 17 '21 18:02 thomasplevy