PlanBCode / hypha

1 stars 8 forks source link

Forbid script tags in wymeditor-editable fields #358

Closed matthijskooijman closed 3 years ago

matthijskooijman commented 3 years ago

Currently, fields that are edited by wymeditor are scrubbed client-side by wymeditor. This prevents users from including non-standard HTML, script tags, etc. However, since this is client-side, this can be bypassed, and users now have the possibility to insert e.g. script tags or other malevolent HTML into the database.

Overall, we consider our users trusted, but in practice it turns out their computers are not. We've seen at least two instances where random script tags were present in article HTML, which I assume means a user's computer or browser is compromised and inserts script tags in all fields that contain HTML just before they are submitted.

I think it would be good to protect against this by rejecting wymeditor fields server-side if they contain a script tag. This is not perfect (there's probably still things that can be done to inject stuff, though if we filter the parsed DOM rather than the HTML text, this would be fairly solid), but at least catches this common case of a virus inserting script tags. Since there is no way a user can normally trigger this, I think it would be acceptable to simply refuse to save in this case (i.e. validation error), maybe with a message like "Script tags are not allowed. Maybe your computer or browser is infected with a virus?".

matthijskooijman commented 3 years ago

I'm taking a stab at this.