CVE-2021-32621: Improper Control of Generation of Code ('Code Injection') (CWE-94)
value: XWiki Platform is a generic wiki platform offering runtime services for applications built on top of it. In versions prior to 12.6.7 and 12.10.3, a user without Script or Programming right is able to execute script requiring privileges by editing gadget titles in the dashboard. The issue has been patched in XWiki 12.6.7, 12.10.3 and 13.0RC1.
CWE description: The product constructs all or part of a code segment using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the syntax or behavior of the intended code segment.
CVSS v3 base score: 8.8
Published: 2021-05-28T21:15Z
Repository: https://github.com/xwiki/xwiki-platform
Language: Java
Files changed: xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/main/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSource.java, xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/test/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSourceTest.java
Commit date: 2021-01-12 15:44:00 +0100
Commit message: XWIKI-17794: Properly interpret velocity in gadget titles

Diff: diff --git a/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/main/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSource.java b/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/main/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSource.java
index 8b8bb063345..15754d12285 100644
--- a/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/main/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSource.java
+++ b/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/main/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSource.java
@@ -42,21 +42,20 @@
 import org.xwiki.rendering.block.GroupBlock;
 import org.xwiki.rendering.block.LinkBlock;
 import org.xwiki.rendering.block.WordBlock;
 import org.xwiki.rendering.block.XDOM;
 import org.xwiki.rendering.executor.ContentExecutor;
-import org.xwiki.rendering.executor.ContentExecutorException;
 import org.xwiki.rendering.listener.reference.ResourceReference;
 import org.xwiki.rendering.listener.reference.ResourceType;
 import org.xwiki.rendering.macro.dashboard.Gadget;
 import org.xwiki.rendering.macro.dashboard.GadgetSource;
-import org.xwiki.rendering.parser.MissingParserException;
-import org.xwiki.rendering.parser.ParseException;
 import org.xwiki.rendering.syntax.Syntax;
 import org.xwiki.rendering.transformation.MacroTransformationContext;
 import org.xwiki.rendering.util.ParserUtils;
 import org.xwiki.security.authorization.AuthorExecutor;
+import org.xwiki.security.authorization.AuthorizationManager;
+import org.xwiki.security.authorization.Right;
 import org.xwiki.velocity.VelocityEngine;
 import org.xwiki.velocity.VelocityManager;
 
 import com.xpn.xwiki.XWiki;
 import com.xpn.xwiki.XWikiContext;
@@ -118,10 +117,13 @@ public class DefaultGadgetSource implements GadgetSource
     private ContentExecutor<MacroTransformationContext> contentExecutor;
 
     @Inject
     private JobProgressManager progress;
 
+    @Inject
+    private AuthorizationManager authorizationManager;
+
     /**
      * Prepare the parser to parse the title and content of the gadget into blocks.
      */
     private ParserUtils parserUtils = new ParserUtils();
 
@@ -192,23 +194,27 @@ private List<Gadget> prepareGadgets(List<BaseObject> objects, Syntax sourceSynta
                 String title = xObject.getStringValue("title");
                 String content = xObject.getLargeStringValue("content");
                 String position = xObject.getStringValue("position");
                 String id = xObject.getNumber() + "";
 
-                // render title with velocity
-                StringWriter writer = new StringWriter();
-                // FIXME: the engine has an issue with $ and # as last character. To test and fix if it happens
-                velocityEngine.evaluate(velocityContext, writer, key, title);
-                String gadgetTitle = writer.toString();
+                String gadgetTitle;
+
+                XWikiDocument ownerDocument = xObject.getOwnerDocument();
+                if (this.authorizationManager.hasAccess(Right.SCRIPT, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference())) {
+                    gadgetTitle =
+                        this.evaluateVelocityTitle(velocityContext, velocityEngine, key, title, ownerDocument);
+                } else {
+                    gadgetTitle = title;
+                }
 
                 // parse both the title and content in the syntax of the transformation context
                 List<Block> titleBlocks =
                     renderGadgetProperty(gadgetTitle, sourceSyntax, xObject.getDocumentReference(),
-                        xObject.getOwnerDocument(), context);
+                        ownerDocument, context);
                 List<Block> contentBlocks =
                     renderGadgetProperty(content, sourceSyntax, xObject.getDocumentReference(),
-                        xObject.getOwnerDocument(), context);
+                        ownerDocument, context);
 
                 // create a gadget will all these and add the gadget to the container of gadgets
                 Gadget gadget = new Gadget(id, titleBlocks, contentBlocks, position);
                 gadget.setTitleSource(title);
                 gadgets.add(gadget);
@@ -220,10 +226,22 @@ private List<Gadget> prepareGadgets(List<BaseObject> objects, Syntax sourceSynta
         }
 
         return gadgets;
     }
 
+    private String evaluateVelocityTitle(VelocityContext velocityContext, VelocityEngine velocityEngine, String key,
+        String title, XWikiDocument ownerDocument) throws Exception
+    {
+        return this.authorExecutor.call(() -> {
+            // render title with velocity
+            StringWriter writer = new StringWriter();
+            // FIXME: the engine has an issue with $ and # as last character. To test and fix if it happens
+            velocityEngine.evaluate(velocityContext, writer, key, title);
+            return writer.toString();
+        }, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference());
+    }
+
     private List<Block> renderGadgetProperty(String content, Syntax sourceSyntax, EntityReference sourceReference,
         XWikiDocument ownerDocument, MacroTransformationContext context)
         throws Exception
     {
         // Ensure that the gadgets are executed with the proper rights.
diff --git a/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/test/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSourceTest.java b/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/test/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSourceTest.java
index aab85fedfc9..74e88065423 100644
--- a/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/test/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSourceTest.java
+++ b/xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/test/java/org/xwiki/rendering/internal/macro/dashboard/DefaultGadgetSourceTest.java
@@ -39,10 +39,12 @@
 import org.xwiki.rendering.executor.ContentExecutor;
 import org.xwiki.rendering.macro.dashboard.Gadget;
 import org.xwiki.rendering.transformation.MacroTransformationContext;
 import org.xwiki.rendering.transformation.TransformationContext;
 import org.xwiki.security.authorization.AuthorExecutor;
+import org.xwiki.security.authorization.AuthorizationManager;
+import org.xwiki.security.authorization.Right;
 import org.xwiki.test.junit5.mockito.ComponentTest;
 import org.xwiki.test.junit5.mockito.InjectMockComponents;
 import org.xwiki.test.junit5.mockito.MockComponent;
 import org.xwiki.test.mockito.MockitoComponentManager;
 import org.xwiki.velocity.VelocityEngine;
@@ -55,10 +57,11 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @ComponentTest
 class DefaultGadgetSourceTest
 {
@@ -70,10 +73,13 @@
     private static final String transformationId = "fooId";
 
     @MockComponent
     private AuthorExecutor authorExecutor;
 
+    @MockComponent
+    private AuthorizationManager authorizationManager;
+
     @Mock
     private DocumentReference documentReference;
 
     @Mock
     private XWikiDocument xWikiDocument;
@@ -97,10 +103,15 @@
     private DocumentReference gadgetClassReference;
 
     @Mock
     private MacroTransformationContext macroTransformationContext;
 
+    @Mock
+    private VelocityEngine velocityEngine;
+
+    private ContentExecutor<MacroTransformationContext> contentExecutor;
+
     @BeforeEach
     void setup(MockitoComponentManager componentManager) throws Exception
     {
         DocumentReferenceResolver<String> currentReferenceResolver =
             componentManager.getInstance(DocumentReferenceResolver.TYPE_STRING, "current");
@@ -121,27 +132,18 @@ void setup(MockitoComponentManager componentManager) throws Exception
         TransformationContext transformationContext = mock(TransformationContext.class);
         when(macroTransformationContext.getTransformationContext()).thenReturn(transformationContext);
         when(transformationContext.getId()).thenReturn(transformationId);
 
         VelocityManager velocityManager = componentManager.getInstance(VelocityManager.class);
-        VelocityEngine velocityEngine = mock(VelocityEngine.class);
         when(velocityManager.getVelocityEngine()).thenReturn(velocityEngine);
-        when(velocityEngine.evaluate(any(), any(), any(), any(String.class))).then((Answer<Void>) invocation -> {
-            Object[] args = invocation.getArguments();
-            StringWriter stringWriter = (StringWriter) args[1];
-            String title = (String) args[3];
-            stringWriter.append(title);
-            return null;
-        });
 
-        AuthorExecutor authorExecutor = componentManager.getInstance(AuthorExecutor.class);
         when(authorExecutor.call(any(), eq(ownerAuthorReference), eq(ownerSourceReference))).then(invocationOnMock -> {
             Callable callable = (Callable) invocationOnMock.getArguments()[0];
             return callable.call();
         });
 
-        ContentExecutor<MacroTransformationContext> contentExecutor =
+        this.contentExecutor =
             componentManager.getInstance(ContentExecutor.TYPE_MACRO_TRANSFORMATION);
         when(contentExecutor.execute(any(), any(), any(), any())).then((Answer<XDOM>) invocationOnMock -> {
             String content = invocationOnMock.getArgument(0);
             XDOM xdom = new XDOM(Collections.singletonList(new WordBlock(content)));
             return xdom;
@@ -160,14 +162,52 @@ void getGadgets() throws Exception
         when(gadgetObject1.getOwnerDocument()).thenReturn(ownerDocument);
         when(gadgetObject1.getStringValue("title")).thenReturn("Gadget 1");
         when(gadgetObject1.getLargeStringValue("content")).thenReturn("Some content");
         when(gadgetObject1.getStringValue("position")).thenReturn("0");
         when(gadgetObject1.getNumber()).thenReturn(42);
+        when(this.authorizationManager.hasAccess(Right.SCRIPT, ownerAuthorReference, ownerSourceReference)).thenReturn(true);
+        when(this.velocityEngine.evaluate(any(), any(), any(), eq("Gadget 1"))).then((Answer<Void>) invocation -> {
+            Object[] args = invocation.getArguments();
+            StringWriter stringWriter = (StringWriter) args[1];
+            String title = "Evaluated velocity version of gadget 1";
+            stringWriter.append(title);
+            return null;
+        });
 
         List<Gadget> gadgets = this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext);
         assertEquals(1, gadgets.size());
         Gadget gadget = gadgets.get(0);
-        assertEquals("Gadget 1", gadget.getTitle().get(0).toString());
+        assertEquals("Evaluated velocity version of gadget 1", gadget.getTitle().get(0).toString());
         assertEquals("Some content", gadget.getContent().get(0).toString());
         assertEquals("42", gadget.getId());
+        verify(this.contentExecutor)
+            .execute(eq("Evaluated velocity version of gadget 1"), any(), any(), any());
+        verify(this.contentExecutor)
+            .execute(eq("Some content"), any(), any(), any());
+    }
+
+    @Test
+    void getGadgetWithoutScriptRight() throws Exception
+    {
+        assertEquals(new ArrayList<>(), this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext));
+
+        BaseObject gadgetObject1 = mock(BaseObject.class);
+        when(xWikiDocument.getXObjects(gadgetClassReference)).thenReturn(Collections.singletonList(gadgetObject1));
+        when(gadgetObject1.getOwnerDocument()).thenReturn(ownerDocument);
+        when(gadgetObject1.getStringValue("title")).thenReturn("Gadget 2");
+        when(gadgetObject1.getLargeStringValue("content")).thenReturn("Some other content");
+        when(gadgetObject1.getStringValue("position")).thenReturn("2");
+        when(gadgetObject1.getNumber()).thenReturn(12);
+        when(this.authorizationManager.hasAccess(Right.SCRIPT, ownerAuthorReference, ownerSourceReference)).thenReturn(false);
+
+        List<Gadget> gadgets = this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext);
+        assertEquals(1, gadgets.size());
+        Gadget gadget = gadgets.get(0);
+        assertEquals("Gadget 2", gadget.getTitle().get(0).toString());
+        assertEquals("Some other content", gadget.getContent().get(0).toString());
+        assertEquals("12", gadget.getId());
+        verify(this.contentExecutor)
+            .execute(eq("Gadget 2"), any(), any(), any());
+        verify(this.contentExecutor)
+            .execute(eq("Some other content"), any(), any(), any());
     }
 }

Vulnerable code: import org.xwiki.rendering.executor.ContentExecutorException;
import org.xwiki.rendering.parser.MissingParserException;
import org.xwiki.rendering.parser.ParseException;
                // render title with velocity
                StringWriter writer = new StringWriter();
                // FIXME: the engine has an issue with $ and # as last character. To test and fix if it happens
                velocityEngine.evaluate(velocityContext, writer, key, title);
                String gadgetTitle = writer.toString();
                        xObject.getOwnerDocument(), context);
                        xObject.getOwnerDocument(), context);
        VelocityEngine velocityEngine = mock(VelocityEngine.class);
        when(velocityEngine.evaluate(any(), any(), any(), any(String.class))).then((Answer<Void>) invocation -> {
            Object[] args = invocation.getArguments();
            StringWriter stringWriter = (StringWriter) args[1];
            String title = (String) args[3];
            stringWriter.append(title);
            return null;
        });
        AuthorExecutor authorExecutor = componentManager.getInstance(AuthorExecutor.class);
        ContentExecutor<MacroTransformationContext> contentExecutor =
        assertEquals("Gadget 1", gadget.getTitle().get(0).toString());
Fixed code: import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.Right;
    @Inject
    private AuthorizationManager authorizationManager;

                String gadgetTitle;

                XWikiDocument ownerDocument = xObject.getOwnerDocument();
                if (this.authorizationManager.hasAccess(Right.SCRIPT, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference())) {
                    gadgetTitle =
                        this.evaluateVelocityTitle(velocityContext, velocityEngine, key, title, ownerDocument);
                } else {
                    gadgetTitle = title;
                }
                        ownerDocument, context);
                        ownerDocument, context);
    private String evaluateVelocityTitle(VelocityContext velocityContext, VelocityEngine velocityEngine, String key,
        String title, XWikiDocument ownerDocument) throws Exception
    {
        return this.authorExecutor.call(() -> {
            // render title with velocity
            StringWriter writer = new StringWriter();
            // FIXME: the engine has an issue with $ and # as last character. To test and fix if it happens
            velocityEngine.evaluate(velocityContext, writer, key, title);
            return writer.toString();
        }, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference());
    }

import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.Right;
import static org.mockito.Mockito.verify;
    @MockComponent
    private AuthorizationManager authorizationManager;

    @Mock
    private VelocityEngine velocityEngine;

    private ContentExecutor<MacroTransformationContext> contentExecutor;

        this.contentExecutor =
        when(this.authorizationManager.hasAccess(Right.SCRIPT, ownerAuthorReference, ownerSourceReference)).thenReturn(true);
        when(this.velocityEngine.evaluate(any(), any(), any(), eq("Gadget 1"))).then((Answer<Void>) invocation -> {
            Object[] args = invocation.getArguments();
            StringWriter stringWriter = (StringWriter) args[1];
            String title = "Evaluated velocity version of gadget 1";
            stringWriter.append(title);
            return null;
        });
        assertEquals("Evaluated velocity version of gadget 1", gadget.getTitle().get(0).toString());
        verify(this.contentExecutor)
            .execute(eq("Evaluated velocity version of gadget 1"), any(), any(), any());
        verify(this.contentExecutor)
            .execute(eq("Some content"), any(), any(), any());
    }

    @Test
    void getGadgetWithoutScriptRight() throws Exception
    {
        assertEquals(new ArrayList<>(), this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext));

        BaseObject gadgetObject1 = mock(BaseObject.class);
        when(xWikiDocument.getXObjects(gadgetClassReference)).thenReturn(Collections.singletonList(gadgetObject1));
        when(gadgetObject1.getOwnerDocument()).thenReturn(ownerDocument);
        when(gadgetObject1.getStringValue("title")).thenReturn("Gadget 2");
        when(gadgetObject1.getLargeStringValue("content")).thenReturn("Some other content");
        when(gadgetObject1.getStringValue("position")).thenReturn("2");
        when(gadgetObject1.getNumber()).thenReturn(12);
        when(this.authorizationManager.hasAccess(Right.SCRIPT, ownerAuthorReference, ownerSourceReference)).thenReturn(false);

        List<Gadget> gadgets = this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext);
        assertEquals(1, gadgets.size());
        Gadget gadget = gadgets.get(0);
        assertEquals("Gadget 2", gadget.getTitle().get(0).toString());
        assertEquals("Some other content", gadget.getContent().get(0).toString());
        assertEquals("12", gadget.getId());
        verify(this.contentExecutor)
            .execute(eq("Gadget 2"), any(), any(), any());
        verify(this.contentExecutor)
            .execute(eq("Some other content"), any(), any(), any());