[xwiki-devs] [Proposal] Restricting the wiki syntax
Hi all, We have some security issues with the wiki syntax : people can use it for including some javascript, as you can pass javascript attributes (onclick, etc...) in links parameters for example. As it is dangerous to let anyone include javascript code, we should at least restrict which attributes unprivileged users could use with the wiki syntax. The question is, should users with PR rights still be able to include Javascript thanks to the syntax ? Either : 1) We restrict the wiki syntax for unprivileged users but give no restriction for users with PR. 2) We restrict the wiki syntax for everybody. To my mind, the wiki syntax is not designed for including javascript, there is the HTML macro and Skin extensions for that, so I'm in favor of 2). But perhaps this is something some of you use often, in which case we should perhaps rather go for solution 1). What do you think ? Thanks, Thomas
Definitely 2, since otherwise the syntax would be inconsistent. The next question is: 3) whitelist: specify which attributes are allowed 4) blacklist: specify which attributes are forbidden I'd go for 3), since the on* attributes vary depending on browser and browser version, and will continue to grow as new HTML/JS APIs appear. 3a) list all currently known HTML attributes (except event ones) 3b) list only the attributes that we want to allow to use 3c) list HTML attributes, but allow all namespaced attributes for extensibility (xwiki: prefix for our own usage, rdf: and rdfs: for semantic stuff, role: for accessibility, etc) For 4), instead of a specific list of attributes, we could blacklist attributes that start with on*. On 06/19/2013 09:23 AM, Thomas Delafosse wrote:
Hi all,
We have some security issues with the wiki syntax : people can use it for including some javascript, as you can pass javascript attributes (onclick, etc...) in links parameters for example. As it is dangerous to let anyone include javascript code, we should at least restrict which attributes unprivileged users could use with the wiki syntax. The question is, should users with PR rights still be able to include Javascript thanks to the syntax ? Either : 1) We restrict the wiki syntax for unprivileged users but give no restriction for users with PR. 2) We restrict the wiki syntax for everybody.
To my mind, the wiki syntax is not designed for including javascript, there is the HTML macro and Skin extensions for that, so I'm in favor of 2). But perhaps this is something some of you use often, in which case we should perhaps rather go for solution 1).
What do you think ?
Thanks,
Thomas
-- Sergiu Dumitriu http://purl.org/net/sergiu
+1 for 2) Besides the security issues, it's not a good practice to have in-line JavaScript as Sergiu pointed out. Regarding whitelisting vs. blacklisting, whatever the solution we need to make sure we support the HTML5 "data-" (custom) attributes. Thanks, Marius On Wed, Jun 19, 2013 at 5:06 PM, Sergiu Dumitriu <sergiu@xwiki.org> wrote:
Definitely 2, since otherwise the syntax would be inconsistent.
The next question is:
3) whitelist: specify which attributes are allowed 4) blacklist: specify which attributes are forbidden
I'd go for 3), since the on* attributes vary depending on browser and browser version, and will continue to grow as new HTML/JS APIs appear.
3a) list all currently known HTML attributes (except event ones) 3b) list only the attributes that we want to allow to use 3c) list HTML attributes, but allow all namespaced attributes for extensibility (xwiki: prefix for our own usage, rdf: and rdfs: for semantic stuff, role: for accessibility, etc)
For 4), instead of a specific list of attributes, we could blacklist attributes that start with on*.
On 06/19/2013 09:23 AM, Thomas Delafosse wrote:
Hi all,
We have some security issues with the wiki syntax : people can use it for including some javascript, as you can pass javascript attributes (onclick, etc...) in links parameters for example. As it is dangerous to let anyone include javascript code, we should at least restrict which attributes unprivileged users could use with the wiki syntax. The question is, should users with PR rights still be able to include Javascript thanks to the syntax ? Either : 1) We restrict the wiki syntax for unprivileged users but give no restriction for users with PR. 2) We restrict the wiki syntax for everybody.
To my mind, the wiki syntax is not designed for including javascript, there is the HTML macro and Skin extensions for that, so I'm in favor of 2). But perhaps this is something some of you use often, in which case we should perhaps rather go for solution 1).
What do you think ?
Thanks,
Thomas
-- Sergiu Dumitriu http://purl.org/net/sergiu _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
On Thu, Jun 20, 2013 at 8:36 AM, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:
+1 for 2) Besides the security issues, it's not a good practice to have in-line JavaScript as Sergiu pointed out.
Regarding whitelisting vs. blacklisting, whatever the solution we need to make sure we support the HTML5 "data-" (custom) attributes.
About HTML 5 we should probably start to have HTML 5 renderer and stop mixing things in a renderer called "xhtml/1.0".
Thanks, Marius
On Wed, Jun 19, 2013 at 5:06 PM, Sergiu Dumitriu <sergiu@xwiki.org> wrote:
Definitely 2, since otherwise the syntax would be inconsistent.
The next question is:
3) whitelist: specify which attributes are allowed 4) blacklist: specify which attributes are forbidden
I'd go for 3), since the on* attributes vary depending on browser and browser version, and will continue to grow as new HTML/JS APIs appear.
3a) list all currently known HTML attributes (except event ones) 3b) list only the attributes that we want to allow to use 3c) list HTML attributes, but allow all namespaced attributes for extensibility (xwiki: prefix for our own usage, rdf: and rdfs: for semantic stuff, role: for accessibility, etc)
+1 for 3c) plus allowing anyone to add more regex in xwiki.properties.
For 4), instead of a specific list of attributes, we could blacklist attributes that start with on*.
On 06/19/2013 09:23 AM, Thomas Delafosse wrote:
Hi all,
We have some security issues with the wiki syntax : people can use it for including some javascript, as you can pass javascript attributes (onclick, etc...) in links parameters for example. As it is dangerous to let anyone include javascript code, we should at least restrict which attributes unprivileged users could use with the wiki syntax. The question is, should users with PR rights still be able to include Javascript thanks to the syntax ? Either : 1) We restrict the wiki syntax for unprivileged users but give no restriction for users with PR. 2) We restrict the wiki syntax for everybody.
To my mind, the wiki syntax is not designed for including javascript, there is the HTML macro and Skin extensions for that, so I'm in favor of 2). But perhaps this is something some of you use often, in which case we should perhaps rather go for solution 1).
What do you think ?
Thanks,
Thomas
-- Sergiu Dumitriu http://purl.org/net/sergiu _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
+1, same reason as Marius. Thanks -Vincent On Jun 20, 2013, at 8:36 AM, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:
+1 for 2) Besides the security issues, it's not a good practice to have in-line JavaScript as Sergiu pointed out.
Regarding whitelisting vs. blacklisting, whatever the solution we need to make sure we support the HTML5 "data-" (custom) attributes.
Thanks, Marius
On Wed, Jun 19, 2013 at 5:06 PM, Sergiu Dumitriu <sergiu@xwiki.org> wrote:
Definitely 2, since otherwise the syntax would be inconsistent.
The next question is:
3) whitelist: specify which attributes are allowed 4) blacklist: specify which attributes are forbidden
I'd go for 3), since the on* attributes vary depending on browser and browser version, and will continue to grow as new HTML/JS APIs appear.
3a) list all currently known HTML attributes (except event ones) 3b) list only the attributes that we want to allow to use 3c) list HTML attributes, but allow all namespaced attributes for extensibility (xwiki: prefix for our own usage, rdf: and rdfs: for semantic stuff, role: for accessibility, etc)
For 4), instead of a specific list of attributes, we could blacklist attributes that start with on*.
On 06/19/2013 09:23 AM, Thomas Delafosse wrote:
Hi all,
We have some security issues with the wiki syntax : people can use it for including some javascript, as you can pass javascript attributes (onclick, etc...) in links parameters for example. As it is dangerous to let anyone include javascript code, we should at least restrict which attributes unprivileged users could use with the wiki syntax. The question is, should users with PR rights still be able to include Javascript thanks to the syntax ? Either : 1) We restrict the wiki syntax for unprivileged users but give no restriction for users with PR. 2) We restrict the wiki syntax for everybody.
To my mind, the wiki syntax is not designed for including javascript, there is the HTML macro and Skin extensions for that, so I'm in favor of 2). But perhaps this is something some of you use often, in which case we should perhaps rather go for solution 1).
What do you think ?
Thanks,
Thomas
+1 for 3c) plus allowing anyone to add more regex in xwiki.properties.
I'm OK for allowing namespaced attributes, but I don't really like the idea of letting people add regex. I think it would be more useful and secure to only let people add extra attributes in xwiki.properties.
On Fri, Jun 21, 2013 at 2:17 PM, Thomas Delafosse <thomas.delafosse@xwiki.com> wrote:
+1 for 3c) plus allowing anyone to add more regex in xwiki.properties.
I'm OK for allowing namespaced attributes, but I don't really like the idea of letting people add regex. I think it would be more useful and secure to only let people add extra attributes in xwiki.properties.
3c) is about supporting prefixes which is the same as regexp for me and if people put bad regex in xwiki.properties it's their fault
_______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
I had in mind to authorize any attribute matching the following regex "^[a-zA-Z]+:[a-zA-Z]+$". With that, any prefixed attribute (without special chars) would be accepted, so I don't see any usecase for enabling some extra regex, while it can be useful to authorize some extra attributes.
Hi all, To achieve the implementation of this solution (see https://github.com/tdelafosse/xwiki-rendering/compare/XSS-protected and https://github.com/tdelafosse/xwiki-platform/compare/XWiki-9151 for more details), I've had to break a few APIs. Here are the changes that make CLIRR complain : 1) I've added a getExtraAttributes() method in the RenderingConfiguration 2) I've added a createSecureRenderer(WikiPrinter) method in the PrintRendererFactory interface, in order to be able to get a "secure" renderer from block renderers 3) I've also had to change XMLWikiPrinter and XHTMLWikiPrinter into interfaces. They are implemented by a "secure" and a "default" XMLWikiPrinter (resp. XHTMLWikiPrinter). The "default" printer being the one defined before. Does this seem OK to you ? Thanks, Thomas On Fri, Jun 21, 2013 at 2:34 PM, Thomas Delafosse < thomas.delafosse@xwiki.com> wrote:
I had in mind to authorize any attribute matching the following regex "^[a-zA-Z]+:[a-zA-Z]+$". With that, any prefixed attribute (without special chars) would be accepted, so I don't see any usecase for enabling some extra regex, while it can be useful to authorize some extra attributes.
On 06/24/2013 10:41 AM, Thomas Delafosse wrote:
Hi all,
To achieve the implementation of this solution (see https://github.com/tdelafosse/xwiki-rendering/compare/XSS-protected and https://github.com/tdelafosse/xwiki-platform/compare/XWiki-9151 for more details), I've had to break a few APIs. Here are the changes that make CLIRR complain : 1) I've added a getExtraAttributes() method in the RenderingConfiguration 2) I've added a createSecureRenderer(WikiPrinter) method in the PrintRendererFactory interface, in order to be able to get a "secure" renderer from block renderers 3) I've also had to change XMLWikiPrinter and XHTMLWikiPrinter into interfaces. They are implemented by a "secure" and a "default" XMLWikiPrinter (resp. XHTMLWikiPrinter). The "default" printer being the one defined before.
Does this seem OK to you ?
Do we need the old printers? I guess that as an independent rendering engine, it would be good to have the ability to have a full wiki->XDOM->XHTML pipeline. As a secure independent rendering engine, it would also be good to have a secure XHTML output, without making the (API) users do extra work to get the security working.
Thanks,
Thomas
On Fri, Jun 21, 2013 at 2:34 PM, Thomas Delafosse < thomas.delafosse@xwiki.com> wrote:
I had in mind to authorize any attribute matching the following regex "^[a-zA-Z]+:[a-zA-Z]+$". With that, any prefixed attribute (without special chars) would be accepted, so I don't see any usecase for enabling some extra regex, while it can be useful to authorize some extra attributes.
-- Sergiu Dumitriu http://purl.org/net/sergiu
Do we need the old printers?
As this is an independent rendering engine, I guess it's better to give the opportunity to people to choose between the default and the secure mode. We can imagine having some usecases where we don't want to clear javascript for example. That's why I think we would need both the printers.
I guess that as an independent rendering engine, it would be good to have the ability to have a full wiki->XDOM->XHTML pipeline.
As a secure independent rendering engine, it would also be good to have a secure XHTML output, without making the (API) users do extra work to get the security working.
Yes that's why I've also introduced secure blockRenderers and renderers.
So it's easy to put them in the API methods (that's what I've done in XWikiDocument.renderXDOM()), and the API users won't have to do some extra work.
On Fri, Jun 21, 2013 at 2:23 PM, Thomas Mortagne <thomas.mortagne@xwiki.com>wrote:
On Fri, Jun 21, 2013 at 2:17 PM, Thomas Delafosse <thomas.delafosse@xwiki.com> wrote:
+1 for 3c) plus allowing anyone to add more regex in xwiki.properties.
I'm OK for allowing namespaced attributes, but I don't really like the idea of letting people add regex. I think it would be more useful and secure to only let people add extra attributes in xwiki.properties.
3c) is about supporting prefixes which is the same as regexp for me and if people put bad regex in xwiki.properties it's their fault
+1
_______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
On 06/19/2013 09:23 AM, Thomas Delafosse wrote:
Hi all,
We have some security issues with the wiki syntax : people can use it for including some javascript, as you can pass javascript attributes (onclick, etc...) in links parameters for example. As it is dangerous to let anyone include javascript code, we should at least restrict which attributes unprivileged users could use with the wiki syntax. The question is, should users with PR rights still be able to include Javascript thanks to the syntax ? Either : 1) We restrict the wiki syntax for unprivileged users but give no restriction for users with PR. 2) We restrict the wiki syntax for everybody.
To my mind, the wiki syntax is not designed for including javascript, there is the HTML macro and Skin extensions for that, so I'm in favor of 2). But perhaps this is something some of you use often, in which case we should perhaps rather go for solution 1).
The {{html}} macro is not supposed to stay, but the official recommended practice is indeed to use skin extensions for any JS/CSS need.
What do you think ?
Thanks,
Thomas
-- Sergiu Dumitriu http://purl.org/net/sergiu
Yes using a whitelist is the only safe way to achieve it. 3.a) is still dangerous, so I'm rather for the 3.b) (or 3.c)) solution. I think we need at least to support this attributes : "alt", "class", "height", "id", "name", "rel", "scope", "style", "target", "title", "width". Of course, I can add others to this list, if you think that they should be supported. Thanks, Thomas On Wed, Jun 19, 2013 at 4:09 PM, Sergiu Dumitriu <sergiu@xwiki.org> wrote:
On 06/19/2013 09:23 AM, Thomas Delafosse wrote:
Hi all,
We have some security issues with the wiki syntax : people can use it for including some javascript, as you can pass javascript attributes (onclick, etc...) in links parameters for example. As it is dangerous to let anyone include javascript code, we should at least restrict which attributes unprivileged users could use with the wiki syntax. The question is, should users with PR rights still be able to include Javascript thanks to the syntax ? Either : 1) We restrict the wiki syntax for unprivileged users but give no restriction for users with PR. 2) We restrict the wiki syntax for everybody.
To my mind, the wiki syntax is not designed for including javascript, there is the HTML macro and Skin extensions for that, so I'm in favor of 2). But perhaps this is something some of you use often, in which case we should perhaps rather go for solution 1).
The {{html}} macro is not supposed to stay, but the official recommended practice is indeed to use skin extensions for any JS/CSS need.
What do you think ?
Thanks,
Thomas
-- Sergiu Dumitriu http://purl.org/net/sergiu _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
participants (6)
-
Denis Gervalle -
Marius Dumitru Florea -
Sergiu Dumitriu -
Thomas Delafosse -
Thomas Mortagne -
Vincent Massol