From d3eaef66fcb4f7551caac5514a934fe013050e92 Mon Sep 17 00:00:00 2001 From: Karel Maxa Date: Thu, 11 Jul 2019 15:43:26 +0200 Subject: [PATCH] Fix infinite loop in role hierarchy resolving Issue: gh-7035 --- .../hierarchicalroles/RoleHierarchyImpl.java | 26 +++++-------------- .../RoleHierarchyImplTests.java | 6 +++++ 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java b/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java index 1bdc0e3fb2..f59f98e63d 100755 --- a/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java +++ b/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java @@ -215,33 +215,19 @@ public class RoleHierarchyImpl implements RoleHierarchy { // iterate over all higher roles from rolesReachableInOneStepMap for (GrantedAuthority role : this.rolesReachableInOneStepMap.keySet()) { - Set rolesToVisitSet = new HashSet<>(); - - if (this.rolesReachableInOneStepMap.containsKey(role)) { - rolesToVisitSet.addAll(this.rolesReachableInOneStepMap.get(role)); - } - + Set rolesToVisitSet = new HashSet<>(this.rolesReachableInOneStepMap.get(role)); Set visitedRolesSet = new HashSet<>(); while (!rolesToVisitSet.isEmpty()) { // take a role from the rolesToVisit set GrantedAuthority aRole = rolesToVisitSet.iterator().next(); rolesToVisitSet.remove(aRole); - visitedRolesSet.add(aRole); - if (this.rolesReachableInOneStepMap.containsKey(aRole)) { - Set newReachableRoles = this.rolesReachableInOneStepMap - .get(aRole); - - // definition of a cycle: you can reach the role you are starting from - if (rolesToVisitSet.contains(role) - || visitedRolesSet.contains(role)) { - throw new CycleInRoleHierarchyException(); - } - else { - // no cycle - rolesToVisitSet.addAll(newReachableRoles); - } + if (!visitedRolesSet.add(aRole) || !this.rolesReachableInOneStepMap.containsKey(aRole)) { + continue; // Already visited role or role with missing hierarchy + } else if (role.equals(aRole)) { + throw new CycleInRoleHierarchyException(); } + rolesToVisitSet.addAll(this.rolesReachableInOneStepMap.get(aRole)); } this.rolesReachableInOneOrMoreStepsMap.put(role, visitedRolesSet); diff --git a/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java b/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java index f5ee43d566..d25983b3ad 100644 --- a/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java +++ b/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java @@ -168,6 +168,12 @@ public class RoleHierarchyImplTests { } catch (CycleInRoleHierarchyException e) { } + + try { + roleHierarchyImpl.setHierarchy("ROLE_C > ROLE_B\nROLE_B > ROLE_A\nROLE_A > ROLE_B"); + fail("Cycle in role hierarchy was not detected!"); + } catch (CycleInRoleHierarchyException e) { + } } @Test