On 25/07/2019 16:49, Marius Dumitru Florea wrote:
On Thu, Jul 25, 2019 at 5:33 PM Simon Urli <[email protected]> wrote:
Hi Marius,
On 25/07/2019 16:22, Marius Dumitru Florea wrote:
On Thu, Jul 25, 2019 at 11:39 AM Simon Urli <[email protected]>
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.
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?
That's also an option, but potentially more breaking.
IMO the decision was already taken a while ago with
https://github.com/xwiki-contrib/syntax-markdown/commit/907dc0630a1b7cad4292c1472deb7ae4d68c66ae
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.
Woops, I messed up with my links sorry, it was this one:
https://github.com/xwiki/xwiki-rendering/commit/907dc0630a1b7cad4292c1472deb7ae4d68c66ae
Again, do you have a real use case where you need HTML macro in comments?
I don't need HTML Macro in comments, but since it has never been
forbidden I assume that lot of users already used comments with html
macro, in which case it would be a breaking change for them to forbid it
now.
Now, if the restricted mode is only used internally for comments AFAIK,
we recently made it available in the API too for the whole document
content (cf https://jira.xwiki.org/browse/XWIKI-16459), and I don't
think in would be a good idea for those to forbid the usage of HTML macro.
Simon
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/544d290a37cf86dff27aa7bc12be3f6a219f1508
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-c33fcb5dca86b155928768dd6e6fbf7eR146
)
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
[email protected]
More about us at http://www.xwiki.com
--
Simon Urli
Software Engineer at XWiki SAS
[email protected]
More about us at http://www.xwiki.com
--
Simon Urli
Software Engineer at XWiki SAS
[email protected]
More about us at http://www.xwiki.com
