[
https://issues.apache.org/jira/browse/OAK-1115?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13804207#comment-13804207
]
Jukka Zitting commented on OAK-1115:
------------------------------------
bq. as soon as a node was really removed the validator checks if "that" node
can be removed but doesn't check for all the child nodes which makes sense IMO.
Consider the following variant of the test case:
{code:java}
@Test
public void testRemoveTreeWithoutPermissionOnChild() throws Exception {
superuser.getNode(childNPath).addNode(nodeName3);
superuser.save();
/* allow READ/WRITE privilege for testUser at 'path' */
allow(path, testUser.getPrincipal(), readWritePrivileges);
/* deny READ/REMOVE property privileges at subtree. */
deny(path, privilegesFromNames(new String[]
{PrivilegeConstants.JCR_REMOVE_NODE}), createGlobRestriction("*/"+nodeName3));
testSession.getNode(childNPath).remove();
try {
testSession.save();
fail("Removing child node must be denied.");
} catch (AccessDeniedException e) {
// success
}
}
{code}
Alternatively, consider the following case where moving a subtree takes it away
from the scope of a glob restriction pattern:
{code:java}
@Test
public void testMoveRemoveSubtree2() throws Exception {
superuser.getNode(childNPath).addNode("parent").addNode("child");
superuser.save();
/* allow READ/WRITE privilege for testUser at 'path' */
allow(path, testUser.getPrincipal(), readWritePrivileges);
/* deny READ/REMOVE property privileges deep within the subtree. */
deny(childNPath,
privilegesFromNames(new String[] { JCR_REMOVE_NODE }),
createGlobRestriction("*/child"));
testSession.move(childNPath + "/parent", childNPath2 + "/dest");
Node dest = testSession.getNode(childNPath2 + "/dest");
dest.getNode("child").remove();
try {
testSession.save();
fail("Removing child node must be denied.");
} catch (AccessDeniedException e) {
// success
}
}
{code}
In both cases, regardless of how the subtree was removed, I think it's
important that the remove permissions are evaluated for the entire subtree
instead of just the parent.
> Remove of Subtree after Move is not subjected to permission validation
> ----------------------------------------------------------------------
>
> Key: OAK-1115
> URL: https://issues.apache.org/jira/browse/OAK-1115
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: core
> Reporter: angela
> Priority: Critical
>
> the following test passes in Jackrabbit-Core but fails in OAK:
> {code}
> @Test
> public void testMoveRemoveSubTree() throws Exception {
> superuser.getNode(childNPath).addNode(nodeName3);
> superuser.save();
> /* allow READ/WRITE privilege for testUser at 'path' */
> givePrivileges(path, privilegesFromNames(new String[]
> {Privilege.JCR_READ, "rep:write"}), Collections.<String, Value>emptyMap());
> /* deny READ/REMOVE property privileges at subtree. */
> withdrawPrivileges(path, privilegesFromNames(new String[]
> {Privilege.JCR_REMOVE_NODE}), Collections.singletonMap("rep:glob",
> superuser.getValueFactory().createValue("*/"+nodeName3)));
> Session testSession = getTestSession();
> assertTrue(testSession.nodeExists(childNPath));
> assertTrue(testSession.hasPermission(childNPath,
> Session.ACTION_REMOVE));
> assertTrue(testSession.hasPermission(childNPath2,
> Session.ACTION_ADD_NODE));
> testSession.move(childNPath, childNPath2 + "/dest");
> Node dest = testSession.getNode(childNPath2 + "/dest");
> dest.getNode(nodeName3).remove();
> try {
> testSession.save();
> fail("Removing child node must be denied.");
> } catch (AccessDeniedException e) {
> // success
> }
> }
> {code}
> this is a critical security issue as it moving around the parent is
> sufficient in order to be able to remove a node that was otherwise not
> removable due to limited permissions.
> Afaik this behavior is caused by a limitation in the Diff process which
> doesn't allow to identify the move and thus makes it impossible to find out
> if that the subtree has been removed.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
