25 Jul
2019
25 Jul
'19
5:26 p.m.
Personally I favor a whitelist for the allowed attributes.
It seems one needs them for wiki syntax like (%
onmouseover="alert('gotcha');" %) , too.
This means we do not only need it for the HTML-Macro, but also in
(nearly all?) other places.
(Then for me it is ok to discuss this for the HTML-Macro first, where it
might be more obvious that one can do ... interesting things with it.)
For the "href" I'd prefer allowing it and check for the
'javascript:'
prefix, but I have to admit that I do not know how to implement this, so
my option counts little when it is about to choose what to implement.
But then, on second thoughts one might do interesting things with e.g.:
{{html clean="false" wiki="false"}}
<a
href="data:application/html;base64,PHNjcmlwdD5hbGVydCgnZ290Y2hhJyk8L3NjcmlwdD4=">click</a>
{{/html}}
where the base64 encoded part decodes to:
<script>alert('gotcha')</script>, so it seems safer to disallow it by
default. (This does not work out of the box, but requires the user to
choose an application to open the link, but still ...)
For the "style" attribute there are some ways to create XSS style
attacks as CSS became more powerful over time; e.g.
https://stackoverflow.com/q/4546591
Thus I am not sure if we should include it in the whitelist by default.
I think the creation of StyleSheetExtensions in a wiki page needs Admin
rights for the same reason already.
Clemens
On 25.07.19 10:54, Thomas Mortagne wrote:
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 <simon.urli(a)xwiki.com> 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