CVE-2023-29523: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: XWiki Platform is a generic wiki platform offering runtime services for applications built on top of it. Any user who can edit their own user profile can execute arbitrary script macros including Groovy and Python macros that allow remote code execution including unrestricted read and write access to all wiki contents. The same vulnerability can also be exploited in other contexts where the `display` method on a document is used to display a field with wiki syntax, for example in applications created using `App Within Minutes`. This has been patched in XWiki 13.10.11, 14.4.8, 14.10.2 and 15.0RC1. There is no workaround apart from upgrading.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2023-04-19T00:15Z
Repository: https://github.com/xwiki/xwiki-platform
Language: Java
Files changed: xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java, xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java
Commit date: 2022-12-19 19:05:31 +0100
Commit message: XWIKI-20327: Escape closing HTML macro in XWikiDocument#display

Diff: diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java
index 9679fadb32804..e77575b268065 100644
--- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java
+++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java
@@ -218,10 +218,12 @@ public class XWikiDocument implements DocumentModelBridge, Cloneable
 {
     private static final Logger LOGGER = LoggerFactory.getLogger(XWikiDocument.class);
 
     private static final String TM_FAILEDDOCUMENTPARSE = "core.document.error.failedParse";
 
+    private static final String CLOSE_HTML_MACRO = "{{/html}}";
+
     /**
      * An attachment waiting to be deleted at next document save.
      *
      * @version $Id$
      * @since 5.2M1
@@ -3883,14 +3885,23 @@ public String display(String fieldname, String type, String pref, BaseObject obj
             // If we're in new rendering engine we want to wrap the HTML returned by displayView() in
             // a {{html/}} macro so that the user doesn't have to do it.
             // We test if we're inside the rendering engine since it's also possible that this display() method is
             // called directly from a template and in this case we only want HTML as a result and not wiki syntax.
             // TODO: find a more generic way to handle html macro because this works only for XWiki 1.0 and XWiki 2.0
-            // Add the {{html}}{{/html}} only when result really contains html since it's not needed for pure text
-            if (isInRenderingEngine && !is10Syntax(wrappingSyntaxId) && HTMLUtils.containsElementText(result)) {
+            // Add the {{html}}{{/html}} only when result really contains html or { which could be part of an XWiki
+            // macro syntax since it's not needed for pure text
+            if (isInRenderingEngine && !is10Syntax(wrappingSyntaxId)
+                && (HTMLUtils.containsElementText(result) || result.indexOf("{") != -1))
+            {
                 result.insert(0, "{{html clean=\"false\" wiki=\"false\"}}");
-                result.append("{{/html}}");
+                // Escape closing HTML macro syntax.
+                int startIndex = 0;
+                // Start searching at the last match to avoid scanning the whole string again.
+                while ((startIndex = result.indexOf(CLOSE_HTML_MACRO, startIndex)) != -1) {
+                    result.replace(startIndex, startIndex + 2, "&#123;&#123;");
+                }
+                result.append(CLOSE_HTML_MACRO);
             }
 
             return result.toString();
         } catch (Exception ex) {
             // TODO: It would better to check if the field exists rather than catching an exception
diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java
index 4d0c6aae41006..0b6c405903cc1 100644
--- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java
+++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java
@@ -57,10 +57,11 @@
 import com.xpn.xwiki.XWikiContext;
 import com.xpn.xwiki.XWikiException;
 import com.xpn.xwiki.api.DocumentSection;
 import com.xpn.xwiki.objects.BaseObject;
 import com.xpn.xwiki.objects.classes.BaseClass;
+import com.xpn.xwiki.objects.classes.PropertyClass;
 import com.xpn.xwiki.objects.classes.TextAreaClass;
 import com.xpn.xwiki.store.XWikiStoreInterface;
 import com.xpn.xwiki.store.XWikiVersioningStoreInterface;
 import com.xpn.xwiki.test.MockitoOldcore;
 import com.xpn.xwiki.test.junit5.mockito.InjectMockitoOldcore;
@@ -74,14 +75,16 @@
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -645,10 +648,34 @@ public void displayTemplate20()
             this.document.display("string", "edit", this.oldcore.getXWikiContext()));
 
         assertEquals("<p>area</p>", this.document.display("area", "view", this.oldcore.getXWikiContext()));
     }
 
+    @Test
+    void displayEscapesClosingHTMLMacro()
+    {
+        this.oldcore.getXWikiContext().put("isInRenderingEngine", true);
+        when(this.xWiki.getCurrentContentSyntaxId(any())).thenReturn("xwiki/2.1");
+        this.document.setSyntax(Syntax.XWIKI_2_0);
+
+        BaseObject object = mock(BaseObject.class);
+        when(object.getOwnerDocument()).thenReturn(this.document);
+
+        BaseClass xClass = mock(BaseClass.class);
+        when(object.getXClass(any())).thenReturn(xClass);
+        PropertyClass propertyInterface = mock(PropertyClass.class);
+        when(xClass.get("mock")).thenReturn(propertyInterface);
+        doAnswer(call -> {
+            call.getArgument(0, StringBuffer.class).append("{{/html}}content{{/html}}");
+            return null;
+        }).when(propertyInterface).displayView(any(StringBuffer.class), eq("mock"), any(String.class), eq(object),
+            anyBoolean(), any(XWikiContext.class));
+
+        assertEquals("{{html clean=\"false\" wiki=\"false\"}}&#123;&#123;/html}}content&#123;&#123;/html}}{{/html}}",
+            this.document.display("mock", "view", object, this.oldcore.getXWikiContext()));
+    }
+
     @Test
     public void convertSyntax() throws XWikiException
     {
         this.document.setSyntax(Syntax.HTML_4_01);
         this.document.setContent("<p>content not in section</p>" + "<h1>header 1</h1><p>header 1 content</p>"

Vulnerable code:             // Add the {{html}}{{/html}} only when result really contains html since it's not needed for pure text
            if (isInRenderingEngine && !is10Syntax(wrappingSyntaxId) && HTMLUtils.containsElementText(result)) {
                result.append("{{/html}}");
Fixed code:     private static final String CLOSE_HTML_MACRO = "{{/html}}";

            // Add the {{html}}{{/html}} only when result really contains html or { which could be part of an XWiki
            // macro syntax since it's not needed for pure text
            if (isInRenderingEngine && !is10Syntax(wrappingSyntaxId)
                && (HTMLUtils.containsElementText(result) || result.indexOf("{") != -1))
            {
                // Escape closing HTML macro syntax.
                int startIndex = 0;
                // Start searching at the last match to avoid scanning the whole string again.
                while ((startIndex = result.indexOf(CLOSE_HTML_MACRO, startIndex)) != -1) {
                    result.replace(startIndex, startIndex + 2, "&#123;&#123;");
                }
                result.append(CLOSE_HTML_MACRO);
import com.xpn.xwiki.objects.classes.PropertyClass;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.doAnswer;
    @Test
    void displayEscapesClosingHTMLMacro()
    {
        this.oldcore.getXWikiContext().put("isInRenderingEngine", true);
        when(this.xWiki.getCurrentContentSyntaxId(any())).thenReturn("xwiki/2.1");
        this.document.setSyntax(Syntax.XWIKI_2_0);

        BaseObject object = mock(BaseObject.class);
        when(object.getOwnerDocument()).thenReturn(this.document);

        BaseClass xClass = mock(BaseClass.class);
        when(object.getXClass(any())).thenReturn(xClass);
        PropertyClass propertyInterface = mock(PropertyClass.class);
        when(xClass.get("mock")).thenReturn(propertyInterface);
        doAnswer(call -> {
            call.getArgument(0, StringBuffer.class).append("{{/html}}content{{/html}}");
            return null;
        }).when(propertyInterface).displayView(any(StringBuffer.class), eq("mock"), any(String.class), eq(object),
            anyBoolean(), any(XWikiContext.class));

        assertEquals("{{html clean=\"false\" wiki=\"false\"}}&#123;&#123;/html}}content&#123;&#123;/html}}{{/html}}",
            this.document.display("mock", "view", object, this.oldcore.getXWikiContext()));
    }