chamilo / chamilo-lms

Chamilo is a learning management system focused on ease of use and accessibility
https://chamilo.org
GNU General Public License v3.0
811 stars 481 forks source link

"on" replaced by "data-cke-671a164ccbf7b-on" in CKEditor #5887

Open ywarnier opened 4 weeks ago

ywarnier commented 4 weeks ago

1.11.28 (just released) came with many security fixes, one of them being to filter "on(event)" words in any HTML edited through CKEditor. However, this filter was too wide and actually replaced all "on" words preceded by a space by something like "data-cke-671a164ccbf7b-on", which is a big issue.

This stems from this commit: https://github.com/chamilo/chamilo-lms/commit/df47eac9b93700bdf3a73e2596e956e14ab1e4f2 which added the "attr_on_filter()" method, which is not strict enough.

We are working on a fix.

ywarnier commented 4 weeks ago

Given it's a filter on attributes, the regular expression should at least ensure it is withing an HTML tag or (because this might not cover enough) ensure that it checks for a given number of known events from the JavaScript language (this is limited because this might change over time) or to just try and recognize a syntax where the event can be declared as any text contained between "on" and "=", but this might also be too wide and trap any text contained between an "on" word and an equal sign.

ywarnier commented 4 weeks ago

The patch above might still cause issue (mostly in the English language) if a text contains some formula that has " on[something] =" in it, but we assume cases like this will be limited.

ywarnier commented 4 weeks ago

The fix is just one line to change in Chamilo's code, but we're preparing a corrective release to make sure this is included.

ywarnier commented 4 weeks ago

In the meantime, try to avoid the "on" word with a space/separation marker in front in anything edited through the WYSIWYG editor.

AngelFQC commented 2 weeks ago

After 3e2582f64f3017b0af0f515ce5200e209a9e4478