Note that this mail is only for the html cleaner. But this white list will also be important to fix https://jira.xwiki.org/browse/XWIKI-9151. On Thu, Jul 25, 2019 at 10:49 AM Simon Urli &lt;simon.urli(a)xwiki.com&gt; wrote: > > > On 25/07/2019 10:39, Simon Urli wrote: >> Hi everyone, >> >> I'm currently working on improving security on XWiki comments. We >> already use a restricted mode in our comments but that does not cover >> every possible case. In order to improve it we should also filter out >> some part of the html when using the html macro. >> >> I propose: >> >> (a) that we use a configurable whitelist of HTML attributes that >> would be allowed in the output HTML: all the other attributes would be >> filtered out. >> >> (b) that the HTML macro is put in restricted mode for users who do >> not have scripting rights. >> >> For (a) I'm hesitating between a whitelist or a blacklist: I assume a >> blacklist would be shorter but there's also more risk of missing >> something. On the contrary a configurable whitelist doesn't prevent >> administrator to accept more than what we give in standard. >> >> A first whitelist could be (taken from: >> https://github.com/xwiki/xwiki-platform/pull/122/files#diff-c33fcb5dca86b15…) >> >> alt, class, height, id, name, rel, scope, style, target, title, width >> >> Note that href is not included in this list for example. > IMO the href not being included in the list is related to the > possibility to write something like: > <a href="javascript:myfunction()"> > > Now I guess we could also detect the "javascript:" prefix in the href > attribute in restricted mode and discard only those, I don't see other > usecase where it could be a problem. > >> WDYT? >> >> Simon >> > -- > Simon Urli > Software Engineer at XWiki SAS > simon.urli(a)xwiki.com > More about us at http://www.xwiki.com

On Thu, Jul 25, 2019 at 11:39 AM Simon Urli &lt;simon.urli(a)xwiki.com&gt; wrote: Is the HTML macro needed in comments? Do you have a real use case for this? Wouldn't it be more simple to completly forbid HTML and script macros in comments?

Thanks, Marius > > I propose: > > (a) that we use a configurable whitelist of HTML attributes that > would be allowed in the output HTML: all the other attributes would be > filtered out. > > (b) that the HTML macro is put in restricted mode for users who do > not have scripting rights. > > For (a) I'm hesitating between a whitelist or a blacklist: I assume a > blacklist would be shorter but there's also more risk of missing > something. On the contrary a configurable whitelist doesn't prevent > administrator to accept more than what we give in standard. > > A first whitelist could be (taken from: > > https://github.com/xwiki/xwiki-platform/pull/122/files#diff-c33fcb5dca86b15… > ) > alt, class, height, id, name, rel, scope, style, target, title, width > > Note that href is not included in this list for example. > > WDYT? > > Simon > > -- > Simon Urli > Software Engineer at XWiki SAS > simon.urli(a)xwiki.com > More about us at http://www.xwiki.com >

On Thu, Jul 25, 2019 at 5:33 PM Simon Urli &lt;simon.urli(a)xwiki.com&gt; wrote: I don't understand why a change in markdown syntax (contrib project) can be considered as decision for the XWiki Project. But anyway, any decision can be revisited.

Again, do you have a real use case where you need HTML macro in comments?

> > which forces to use html macro with clean option when used in restricted > mode, but does not prevent to use html macro at all. Contrary to the > script macro which have all been disabled on the following commit: > > https://github.com/xwiki/xwiki-platform/commit/544d290a37cf86dff27aa7bc12be… > > Simon >> >> Thanks, >> Marius >> >> >>> >>> I propose: >>> >>> (a) that we use a configurable whitelist of HTML attributes that >>> would be allowed in the output HTML: all the other attributes would be >>> filtered out. >>> >>> (b) that the HTML macro is put in restricted mode for users who do >>> not have scripting rights. >>> >>> For (a) I'm hesitating between a whitelist or a blacklist: I assume a >>> blacklist would be shorter but there's also more risk of missing >>> something. On the contrary a configurable whitelist doesn't prevent >>> administrator to accept more than what we give in standard. >>> >>> A first whitelist could be (taken from: >>> >>> > https://github.com/xwiki/xwiki-platform/pull/122/files#diff-c33fcb5dca86b15… >>> ) >>> alt, class, height, id, name, rel, scope, style, target, title, width >>> >>> Note that href is not included in this list for example. >>> >>> WDYT? >>> >>> Simon >>> >>> -- >>> Simon Urli >>> Software Engineer at XWiki SAS >>> simon.urli(a)xwiki.com >>> More about us at http://www.xwiki.com >>> > > -- > Simon Urli > Software Engineer at XWiki SAS > simon.urli(a)xwiki.com > More about us at http://www.xwiki.com >