search

Automattic / liveblog

Liveblogging done right. Using WordPress.
https://wordpress.org/plugins/liveblog/
308 stars 122 forks source link

Review usage of dangerouslySetInnerHTML #526

Open nickdaugherty opened 6 years ago

nickdaugherty commented 6 years ago

@uxcitizen noticed that there are several spots in this plugin using React's dangerouslySetInnerHTML.

https://github.com/Automattic/liveblog/search?q=dangerouslysetinnerhtml&unscoped_q=dangerouslysetinnerhtml

How many of those can we get rid of with refactoring?

Obviously some places are showing pre-rendered HTML from the server. In those cases, we should be sanitizing the HTML to strip out any unexpected or disallowed tags and attributes before passing it to dangerouslySetInnerHTML to reduce the potential for abuse.

Ideally we have 0 of these calls, but for the ones we can't get away from, we should be sure everything is sanitized as far as possible.

cain commented 6 years ago

Seems like all of the WordPress endpoints return HTML, it's not possible without using dangerouslySetInnerHTML

But you are right, some sanitizing needs to be done. https://github.com/cure53/DOMPurify works well.

davidsword commented 6 years ago

I'll look into this!

cain commented 6 years ago

@davidsword looks really good!