Configuration to sanitize <script> tags in CMS content

[chiubaka]

Tried asking about this on Gitter, but no response...

I run a site that uses this gem as a simple CMS system. We had a routine security review a few weeks ago, and a security expert flagged that he can create pages with arbitrary scripts by entering script tags into the WSIWYG code.

While I'm aware that Comfy has a separate field in layouts allowing admins to add scripts to pages, the security expert's concern is that leaving things this way could leave the system vulnerable to an administrator unwittingly creating an exploitable situation without having meant to.

Expected behavior

Script tags in WSIWYG or other CMS text content should be escaped or sanitized. OR a configuration option should be provided to allow for this.

Actual behavior

Script tags are rendered as is, allowing for running of arbitrary scripts in a CMS page.

[GBH]

Hi @chiubaka

It's been a while since I looked into this, but you can change default options for wysiwyg by overwriting this file: https://github.com/comfy/comfortable-mexican-sofa/blob/master/app/assets/javascripts/comfy/admin/cms/wysiwyg.js

I'm a bit surprised that i doesn't remove script tags. Seems like it should not allow those by default: https://github.com/comfy/comfortable-mexican-sofa/blob/master/app/assets/javascripts/comfy/vendor/redactor.js#L196

[chiubaka]

Hey @GBH,

Thanks for your response, and apologies for the delay in mine.

I slightly mis-spoke in describing the issue here. Here's the thread between our security consultant and me illustrating the attack vector:

So, you're correct, the WSIWYG widget is attempting to sanitize the input, however, it is possible to circumvent this. The measure that really needs to be in place here is the CMS itself escaping the scripts before the content is pulled from the DB and displayed on the page. Or really just some form of backend protection here. Right now security of the system depends on a well-behaved client.

I don't suppose there are any existing options lying around to do this?

Best,
Daniel

[GBH]

One thing I can suggest is to add a before_save callback on the https://github.com/comfy/comfortable-mexican-sofa/blob/master/app/models/comfy/cms/fragment.rb (and snippet model too maybe) that sanitizes saved content.

[chiubaka]

Interesting. A before_save callback could work. Alternatively a sanitize call before rendering the fragment content could work. The latter might be a little more flexible/configurable, since the data saved to the database doesn't change, but how we pre-process it before rendering does.

Are these changes you'd feel comfortable with finding their into the main codebase?