Description

Motivation

We've discovered and fixed a lot of security issues recently. Many of them are based on XWiki syntax injection and usually the most significant impact is remote code execution through injected script macros. What's common to almost all these possible attacks is that a script macro that is either part of a document authored by user without script/programming right or part of the request is executed in the context of a document authored by a user with programming right. So one could wonder: couldn't we just prevent this kind of attack once and for all? The goal of this is most likely not something that would be included or enabled by default but rather an extension that you could enable if you can live with the restrictions it implies and want the additional security. All of this should be considered an additional defense in depth and not as a replacement for actually fixing these vulnerabilities. It could also be a good protection in case an XWiki instance, e.g., cannot be updated in time and to prevent zero-day exploits.

Implementation

The idea would be the following:

1. Create an index of a cryptographic hash of the content of all script macros that are contained in the current version of a document together with the author and reference of that document. We would search for script macros in the original XDOM of the content, but also of all other properties that can contain XWiki syntax in a document save event handler. In this search, also the content of “simple” macros should be considered similar to, e.g., link refactoring.
2. When a script macro shall be executed, check in the index if the content of this script macro is registered and if its author has script or programming right. For the search, also filter on document reference and accept the current secure document’s or context document’s or XDOM’s source reference. Deny the execution if the script macro is unknown or its author doesn’t have script right or the current context author has programming right but the script macro’s author doesn’t have programing right. If the macro's author has just script right, one could additionally check that the context author is indeed the same as script macro's author.

This would prevent the execution of any script macro that is not contained in the current version of a document that is authored by a user that has at least the same rights as the current context author. This idea is similar to the idea of signed scripts, except that we would only ever consider the script macros contained in the current version of any document as signed and there wouldn’t be any actual signatures. This has the advantage that we don’t need to take care to invalidate signatures that were erroneously put on insecure scripts that could, e.g., disclose information.

Impact

This breaks (at least):

1. Script macros that are dynamically generated, e.g., for an async, cache or job macro or that are dynamically generated on the server side.
2. Changed script macros in the WYSIWYG editor or the preview (or all script macros in the case the previous author didn’t have script right)
3. Changed script macros in old revisions, but they have already been disabled as part of fixing XWIKI-20594. In fact, we could possibly undo this restriction if we implemented dynamic script restrictions.

XSS

While restricting dynamic scripts as proposed above would prevent direct remote code execution, in almost all cases an XWiki syntax injection vulnerability would also allow XSS. We usually don’t mention this as remote code execution allows even more than just XSS. To prevent XSS, we could add a strict Content Security Policy (CSP) and disable inline scripts. If we’re using inline scripts for certain cases, we could allow them by either adding a random nonce to them or adding their hash in the header. This might not be possible transparently without defeating the purpose of the CSP, but an idea could be to make either the nonce available in script macros, or to add an API to register the hash of a script for the CSP header as the above-mentioned checks should limit attacks on script macros.