Skip to content
Snippets Groups Projects
Commit 1f61d134 authored by Thomas Mortagne's avatar Thomas Mortagne
Browse files

XWIKI-12437: Unrestricted Groovy execution via validation

parent 13609e64
No related branches found
No related tags found
No related merge requests found
...@@ -39,6 +39,8 @@ ...@@ -39,6 +39,8 @@
import org.xwiki.model.reference.DocumentReferenceResolver; import org.xwiki.model.reference.DocumentReferenceResolver;
import org.xwiki.model.reference.EntityReference; import org.xwiki.model.reference.EntityReference;
import org.xwiki.model.reference.SpaceReference; import org.xwiki.model.reference.SpaceReference;
import org.xwiki.security.authorization.ContextualAuthorizationManager;
import org.xwiki.security.authorization.Right;
import com.google.common.base.Objects; import com.google.common.base.Objects;
import com.xpn.xwiki.XWikiContext; import com.xpn.xwiki.XWikiContext;
...@@ -94,6 +96,8 @@ public class BaseClass extends BaseCollection<DocumentReference> implements Clas ...@@ -94,6 +96,8 @@ public class BaseClass extends BaseCollection<DocumentReference> implements Clas
*/ */
private DocumentReferenceResolver<String> currentMixedDocumentReferenceResolver; private DocumentReferenceResolver<String> currentMixedDocumentReferenceResolver;
private DocumentReferenceResolver<String> currentDocumentReferenceResolver;
private DocumentReferenceResolver<String> getCurrentMixedDocumentReferenceResolver() private DocumentReferenceResolver<String> getCurrentMixedDocumentReferenceResolver()
{ {
if (this.currentMixedDocumentReferenceResolver == null) { if (this.currentMixedDocumentReferenceResolver == null) {
...@@ -104,6 +108,22 @@ private DocumentReferenceResolver<String> getCurrentMixedDocumentReferenceResolv ...@@ -104,6 +108,22 @@ private DocumentReferenceResolver<String> getCurrentMixedDocumentReferenceResolv
return this.currentMixedDocumentReferenceResolver; return this.currentMixedDocumentReferenceResolver;
} }
/**
* Used to resolve a string into a proper Document Reference using the current document's reference to fill the
* blanks.
*
* @since 7.2M3
*/
private DocumentReferenceResolver<String> getCurrentDocumentReferenceResolver()
{
if (this.currentDocumentReferenceResolver == null) {
this.currentDocumentReferenceResolver =
Utils.getComponent(DocumentReferenceResolver.TYPE_STRING, "current");
}
return this.currentDocumentReferenceResolver;
}
@Override @Override
public DocumentReference getReference() public DocumentReference getReference()
{ {
...@@ -1192,6 +1212,14 @@ public boolean validateObject(BaseObject obj, XWikiContext context) throws XWiki ...@@ -1192,6 +1212,14 @@ public boolean validateObject(BaseObject obj, XWikiContext context) throws XWiki
private boolean executeValidationScript(BaseObject obj, String validationScript, XWikiContext context) private boolean executeValidationScript(BaseObject obj, String validationScript, XWikiContext context)
{ {
try { try {
ContextualAuthorizationManager authorization = Utils.getComponent(ContextualAuthorizationManager.class);
DocumentReference validationScriptReference =
getCurrentDocumentReferenceResolver().resolve(validationScript, getDocumentReference());
// Make sure target document is allowed to execute Groovy
// TODO: this check should probably be right in XWiki#parseGroovyFromPage
authorization.checkAccess(Right.PROGRAM, validationScriptReference);
XWikiValidationInterface validObject = XWikiValidationInterface validObject =
(XWikiValidationInterface) context.getWiki().parseGroovyFromPage(validationScript, context); (XWikiValidationInterface) context.getWiki().parseGroovyFromPage(validationScript, context);
......
...@@ -1368,9 +1368,10 @@ private void copyDocumentAndAssertTitle(DocumentReference oldReference, String o ...@@ -1368,9 +1368,10 @@ private void copyDocumentAndAssertTitle(DocumentReference oldReference, String o
} }
@Test @Test
public void testValidateWithoutPR() throws XWikiException, AccessDeniedException public void testValidate() throws XWikiException, AccessDeniedException
{ {
this.document.setValidationScript("validationScript"); this.document.setValidationScript("validationScript");
this.baseClass.setValidationScript("validationScript");
when(this.oldcore.getMockXWiki().parseGroovyFromPage("validationScript", this.oldcore.getXWikiContext())) when(this.oldcore.getMockXWiki().parseGroovyFromPage("validationScript", this.oldcore.getXWikiContext()))
.thenReturn(new XWikiValidationInterface() .thenReturn(new XWikiValidationInterface()
...@@ -1388,11 +1389,17 @@ public boolean validateDocument(XWikiDocument doc, XWikiContext context) ...@@ -1388,11 +1389,17 @@ public boolean validateDocument(XWikiDocument doc, XWikiContext context)
} }
}); });
// With PR
assertTrue(this.document.validate(this.oldcore.getXWikiContext())); assertTrue(this.document.validate(this.oldcore.getXWikiContext()));
assertTrue(this.baseClass.validateObject(this.baseObject, this.oldcore.getXWikiContext()));
// Without PR
doThrow(AccessDeniedException.class).when(this.oldcore.getMockContextualAuthorizationManager()).checkAccess( doThrow(AccessDeniedException.class).when(this.oldcore.getMockContextualAuthorizationManager()).checkAccess(
Right.PROGRAM, new DocumentReference("wiki", "space", "validationScript")); Right.PROGRAM, new DocumentReference("wiki", "space", "validationScript"));
assertFalse(this.document.validate(this.oldcore.getXWikiContext())); assertFalse(this.document.validate(this.oldcore.getXWikiContext()));
assertFalse(this.baseClass.validateObject(this.baseObject, this.oldcore.getXWikiContext()));
} }
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment