Skip to content

Commit 67b8b20

Browse files
committed
Prevent circular ACL parent relationships
Signed-off-by: 98001yash <yashchauhan.gaya@gmail.com>
1 parent 6ed1545 commit 67b8b20

2 files changed

Lines changed: 32 additions & 1 deletion

File tree

acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,11 @@ public void setOwner(Sid newOwner) {
247247
@Override
248248
public void setParent(@Nullable Acl newParent) {
249249
this.aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
250-
Assert.isTrue(newParent == null || !newParent.equals(this), "Cannot be the parent of yourself");
250+
Acl parent = newParent;
251+
while (parent != null) {
252+
Assert.isTrue(parent != this, "Cannot create a circular parent relationship");
253+
parent = parent.getParentAcl();
254+
}
251255
this.parentAcl = newParent;
252256
}
253257

acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,33 @@ public void gettersAndSettersAreConsistent() {
391391
assertThat(new PrincipalSid("ben")).isEqualTo(acl.getOwner());
392392
}
393393

394+
@Test
395+
public void setParentRejectsAncestor() {
396+
MutableAcl parentAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 101), 101,
397+
this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe"));
398+
MutableAcl childAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 102), 102,
399+
this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe"));
400+
401+
childAcl.setParent(parentAcl);
402+
403+
assertThatIllegalArgumentException().isThrownBy(() -> parentAcl.setParent(childAcl));
404+
}
405+
406+
@Test
407+
public void setParentRejectsIndirectAncestor() {
408+
MutableAcl grandParentAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 101), 101,
409+
this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe"));
410+
MutableAcl parentAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 102), 102,
411+
this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe"));
412+
MutableAcl childAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 103), 103,
413+
this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe"));
414+
415+
parentAcl.setParent(grandParentAcl);
416+
childAcl.setParent(parentAcl);
417+
418+
assertThatIllegalArgumentException().isThrownBy(() -> grandParentAcl.setParent(childAcl));
419+
}
420+
394421
@Test
395422
public void isSidLoadedBehavesAsExpected() {
396423
List<Sid> loadedSids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED"));

0 commit comments

Comments
 (0)