XWiki Platform's Groovy jobs check the wrong author, allowing remote code execution

@@ -34,7 +34,7 @@
   <properties>
     <!-- Name to display by the Extension Manager -->
     <xwiki.extension.name>Scheduler API</xwiki.extension.name>
-    <xwiki.jacoco.instructionRatio>0.00</xwiki.jacoco.instructionRatio>
+    <xwiki.jacoco.instructionRatio>0.08</xwiki.jacoco.instructionRatio>
   </properties>
   <dependencies>
     <dependency>
@@ -50,6 +50,19 @@
       <groupId>javax.servlet</groupId>
       <artifactId>javax.servlet-api</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.xwiki.platform</groupId>
+      <artifactId>xwiki-platform-oldcore</artifactId>
+      <version>${project.version}</version>
+      <type>test-jar</type>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.xwiki.commons</groupId>
+      <artifactId>xwiki-commons-tool-test-component</artifactId>
+      <version>${commons.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
   <build>
     <plugins>

@@ -19,13 +19,20 @@
  */
 package com.xpn.xwiki.plugin.scheduler;
 
+import java.util.Objects;
+
 import org.codehaus.groovy.control.CompilationFailedException;
 import org.quartz.JobDataMap;
 import org.quartz.JobExecutionContext;
 import org.quartz.JobExecutionException;
+import org.xwiki.security.authorization.AccessDeniedException;
+import org.xwiki.security.authorization.ContextualAuthorizationManager;
+import org.xwiki.security.authorization.Right;
 
+import com.xpn.xwiki.XWikiException;
 import com.xpn.xwiki.doc.XWikiDocument;
 import com.xpn.xwiki.objects.BaseObject;
+import com.xpn.xwiki.web.Utils;
 
 import groovy.lang.Binding;
 import groovy.lang.GroovyShell;
@@ -64,32 +71,48 @@ protected void executeJob(JobExecutionContext jobContext) throws JobExecutionExc
             BaseObject object = (BaseObject) data.get("xjob");
 
             // Force context document
-            XWikiDocument jobDocument = getXWikiContext().getWiki().getDocument(object.getName(), getXWikiContext());
+            XWikiDocument jobDocument = object.getOwnerDocument();
+            if (jobDocument == null) {
+                jobDocument = getXWikiContext().getWiki().getDocument(object.getDocumentReference(), getXWikiContext());
+            }
+
             getXWikiContext().setDoc(jobDocument);
-            getXWikiContext().put("sdoc", jobDocument);
+            XWikiDocument secureDocument;
+            // Make sure that the secure document has the correct content author.
+            if (!Objects.equals(jobDocument.getAuthors().getContentAuthor(),
+                jobDocument.getAuthors().getEffectiveMetadataAuthor()))
+            {
+                secureDocument = jobDocument.clone();
+                secureDocument.getAuthors().setContentAuthor(jobDocument.getAuthors().getEffectiveMetadataAuthor());
+            } else {
+                secureDocument = jobDocument;
+            }
+            getXWikiContext().put("sdoc", secureDocument);
 
-            if (getXWikiContext().getWiki().getRightService().hasProgrammingRights(getXWikiContext())) {
+            ContextualAuthorizationManager contextualAuthorizationManager =
+                Utils.getComponent(ContextualAuthorizationManager.class);
+            contextualAuthorizationManager.checkAccess(Right.PROGRAM);
 
-                // Make the Job execution data available to the Groovy script
-                Binding binding = new Binding(data.getWrappedMap());
+            // Make the Job execution data available to the Groovy script
+            Binding binding = new Binding(data.getWrappedMap());
 
-                // Set the right instance of XWikiContext
-                binding.setProperty("context", getXWikiContext());
-                binding.setProperty("xcontext", getXWikiContext());
-                data.put("xwiki", new com.xpn.xwiki.api.XWiki(getXWikiContext().getWiki(), getXWikiContext()));
+            // Set the right instance of XWikiContext
+            binding.setProperty("context", getXWikiContext());
+            binding.setProperty("xcontext", getXWikiContext());
+            data.put("xwiki", new com.xpn.xwiki.api.XWiki(getXWikiContext().getWiki(), getXWikiContext()));
 
-                // Execute the Groovy script
-                GroovyShell shell = new GroovyShell(Thread.currentThread().getContextClassLoader(), binding);
-                shell.evaluate(object.getLargeStringValue("script"));
-            } else {
-                throw new JobExecutionException("The user [" + getXWikiContext().getUser() + "] didn't have "
-                    + "programming rights when the job [" + jobContext.getJobDetail().getKey() + "] was scheduled.");
-            }
+            // Execute the Groovy script
+            GroovyShell shell = new GroovyShell(Thread.currentThread().getContextClassLoader(), binding);
+            shell.evaluate(object.getLargeStringValue("script"));
         } catch (CompilationFailedException e) {
             throw new JobExecutionException(
                 "Failed to execute script for job [" + jobContext.getJobDetail().getKey() + "]", e, true);
-        } catch (Exception e) {
-            e.printStackTrace();
+        } catch (XWikiException e) {
+            throw new JobExecutionException("Failed to load the document containing the job ["
+                + jobContext.getJobDetail().getKey() + "]", e, true);
+        } catch (AccessDeniedException e) {
+            throw new JobExecutionException("Executing the job [" + jobContext.getJobDetail().getKey() + "] failed "
+                + "due to insufficient rights.", e, true);
         }
     }
 }

@@ -0,0 +1,157 @@
+/*
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package com.xpn.xwiki.plugin.scheduler;
+
+import java.util.function.Consumer;
+
+import javax.inject.Inject;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.Mock;
+import org.quartz.JobDataMap;
+import org.quartz.JobDetail;
+import org.quartz.JobExecutionContext;
+import org.quartz.JobExecutionException;
+import org.quartz.JobKey;
+import org.xwiki.context.Execution;
+import org.xwiki.context.internal.DefaultExecution;
+import org.xwiki.context.internal.DefaultExecutionContextManager;
+import org.xwiki.model.reference.DocumentReference;
+import org.xwiki.security.authorization.AccessDeniedException;
+import org.xwiki.security.authorization.Right;
+import org.xwiki.test.annotation.ComponentList;
+import org.xwiki.user.GuestUserReference;
+import org.xwiki.user.UserReference;
+
+import com.xpn.xwiki.XWikiContext;
+import com.xpn.xwiki.XWikiException;
+import com.xpn.xwiki.doc.XWikiDocument;
+import com.xpn.xwiki.internal.ReadOnlyXWikiContextProvider;
+import com.xpn.xwiki.objects.BaseObject;
+import com.xpn.xwiki.plugin.scheduler.internal.SchedulerJobClassDocumentInitializer;
+import com.xpn.xwiki.test.MockitoOldcore;
+import com.xpn.xwiki.test.junit5.mockito.OldcoreTest;
+import com.xpn.xwiki.test.reference.ReferenceComponentList;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit test for {@link GroovyJob}.
+ *
+ * @version $Id$
+ */
+@OldcoreTest
+@ComponentList({
+    SchedulerJobClassDocumentInitializer.class,
+    DefaultExecutionContextManager.class,
+    DefaultExecution.class,
+    ReadOnlyXWikiContextProvider.class
+})
+@ReferenceComponentList
+class GroovyJobTest
+{
+    private static final String WIKI_NAME = "xwiki";
+
+    @Mock
+    private Consumer<UserReference> feedbackFunction;
+
+    @Inject
+    private Execution execution;
+
+    @ParameterizedTest
+    @ValueSource(booleans = { true, false })
+    void programmingRightCheck(boolean allowExecution, MockitoOldcore mockitoOldcore) throws XWikiException,
+        JobExecutionException, AccessDeniedException
+    {
+        GroovyJob job = new GroovyJob();
+
+        // Mock the job context.
+        JobExecutionContext context = mock(JobExecutionContext.class);
+        JobDetail jobDetail = mock(JobDetail.class);
+        when(context.getJobDetail()).thenReturn(jobDetail);
+        when(jobDetail.getKey()).thenReturn(new JobKey("Test Job"));
+        JobDataMap jobDataMap = new JobDataMap();
+        when(jobDetail.getJobDataMap()).thenReturn(jobDataMap);
+
+        // Create a new context to ensure that the context from the job is used.
+        XWikiContext testContext = new XWikiContext();
+        jobDataMap.put("context", testContext);
+        testContext.setWiki(mockitoOldcore.getSpyXWiki());
+        testContext.setUserReference(new DocumentReference(WIKI_NAME, "XWiki", "ContextUser"));
+
+        // Create a mock function that we set on the context and that will then be executed by the job.
+        testContext.put("feedbackFunction", this.feedbackFunction);
+
+        // Create the job document.
+        XWikiDocument jobDocument = new XWikiDocument(new DocumentReference(WIKI_NAME, "Scheduler", "Job"));
+        // Set different authors to verify that the correct author is used.
+        UserReference contentAuthor = mock(UserReference.class);
+        UserReference jobAuthor = mock(UserReference.class);
+        jobDocument.getAuthors().setContentAuthor(contentAuthor);
+        jobDocument.getAuthors().setOriginalMetadataAuthor(GuestUserReference.INSTANCE);
+        jobDocument.getAuthors().setEffectiveMetadataAuthor(jobAuthor);
+        BaseObject xObject = jobDocument.newXObject(SchedulerJobClassDocumentInitializer.XWIKI_JOB_CLASSREFERENCE,
+            mockitoOldcore.getXWikiContext());
+        xObject.setLargeStringValue("script",
+            "xcontext.get('feedbackFunction')"
+                + ".accept(xcontext.get(com.xpn.xwiki.doc.XWikiDocument.CKEY_SDOC).getAuthors().getContentAuthor())");
+        mockitoOldcore.getSpyXWiki().saveDocument(jobDocument, mockitoOldcore.getXWikiContext());
+        jobDataMap.put("xjob", xObject);
+
+        doAnswer(invocationOnMock -> {
+            XWikiContext xWikiContext =
+                (XWikiContext) this.execution.getContext().getProperty(XWikiContext.EXECUTIONCONTEXT_KEY);
+            assertEquals(jobDocument, xWikiContext.getDoc());
+            XWikiDocument sDocument = (XWikiDocument) xWikiContext.get(XWikiDocument.CKEY_SDOC);
+            assertEquals(jobAuthor, sDocument.getAuthors().getContentAuthor());
+            assertNotEquals(contentAuthor, sDocument.getAuthors().getContentAuthor());
+
+            if (!allowExecution) {
+                throw new AccessDeniedException(Right.PROGRAM, new DocumentReference(WIKI_NAME, "XWiki", "JobAuthor"),
+                    sDocument.getDocumentReference());
+            }
+            return null;
+        }).when(mockitoOldcore.getMockContextualAuthorizationManager()).checkAccess(Right.PROGRAM);
+
+        if (allowExecution) {
+            job.execute(context);
+            verify(this.feedbackFunction).accept(jobAuthor);
+        } else {
+            JobExecutionException jobExecutionException =
+                assertThrows(JobExecutionException.class, () -> job.execute(context));
+            verify(this.feedbackFunction, never()).accept(any());
+            assertEquals("Executing the job [DEFAULT.Test Job] failed due to insufficient rights.",
+                jobExecutionException.getMessage());
+            assertEquals("Access denied when checking [programming] access to [xwiki:Scheduler.Job] "
+                + "for user [xwiki:XWiki.JobAuthor]", jobExecutionException.getCause().getMessage());
+        }
+
+        verify(mockitoOldcore.getMockContextualAuthorizationManager()).checkAccess(Right.PROGRAM);
+    }
+}