fix: prevent cyclic traversals in esc13 post processing

packages/go/analysis/ad/esc13.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"sync"
 
+	"github.com/RoaringBitmap/roaring"
 	"github.com/specterops/bloodhound/analysis"
 	"github.com/specterops/bloodhound/analysis/impact"
 	"github.com/specterops/bloodhound/dawgs/cardinality"
@@ -72,37 +73,47 @@ func PostADCSESC13(ctx context.Context, tx graph.Transaction, outC chan<- analys
 }
 
 func groupIsContainedOrTrusted(tx graph.Transaction, group, domain *graph.Node) bool {
-	var matchFound bool
-	if err := ops.Traversal(tx, ops.TraversalPlan{
-		Root:      group,
-		Direction: graph.DirectionInbound,
-		BranchQuery: func() graph.Criteria {
-			return query.KindIn(query.Relationship(), ad.Contains, ad.TrustedBy)
-		},
-		PathFilter: func(ctx *ops.TraversalContext, segment *graph.PathSegment) bool {
-			return segment.Node.Kinds.ContainsOneOf(ad.Domain)
-		},
-	}, func(ctx *ops.TraversalContext, segment *graph.PathSegment) error {
-		//Check to make sure that this segment contains our target domain id
-		segment.WalkReverse(func(nextSegment *graph.PathSegment) bool {
-			if nextSegment.Node.ID == domain.ID {
-				matchFound = true
-				return false
-			}
+	var (
+		matchFound    = false
+		visitedBitmap = roaring.New()
+		traversalPlan = ops.TraversalPlan{
+			Root:      group,
+			Direction: graph.DirectionInbound,
+			ExpansionFilter: func(segment *graph.PathSegment) bool {
+				return visitedBitmap.CheckedAdd(segment.Node.ID.Uint32())
+			},
+			BranchQuery: func() graph.Criteria {
+				return query.KindIn(query.Relationship(), ad.Contains, ad.TrustedBy)
+			},
+			PathFilter: func(ctx *ops.TraversalContext, segment *graph.PathSegment) bool {
+				return segment.Node.Kinds.ContainsOneOf(ad.Domain)
+			},
+		}
+		pathVisitor = func(ctx *ops.TraversalContext, segment *graph.PathSegment) error {
+			//Check to make sure that this segment contains our target domain id
+			segment.WalkReverse(func(nextSegment *graph.PathSegment) bool {
+				if nextSegment.Node.ID == domain.ID {
+					matchFound = true
+					return false
+				}
 
-			if !nextSegment.Node.Kinds.ContainsOneOf(ad.Domain) {
-				return false
-			}
+				if !nextSegment.Node.Kinds.ContainsOneOf(ad.Domain) {
+					return false
+				}
 
-			return true
-		})
+				return true
+			})
 
-		return nil
-	}); err != nil {
-		return false
-	} else {
-		return matchFound
+			return nil
+		}
+	)
+
+	if err := ops.Traversal(tx, traversalPlan, pathVisitor); err != nil {
+		log.Debugf("groupIsContainedOrTrusted traversal error: %v", err)
 	}
+
+	return matchFound
+
 }
 
 func isCertTemplateValidForESC13(ct *graph.Node) (bool, error) {

packages/go/analysis/ad/post.go

@@ -551,14 +551,3 @@ func ProcessRDPWithUra(tx graph.Transaction, rdpLocalGroup *graph.Node, computer
 		return rdpEntities, nil
 	}
 }
-
-func ExpandAllEnrollers(ctx context.Context, db graph.Database) (impact.PathAggregator, error) {
-	log.Infof("Expanding all cert template and enterprise ca enrollers")
-
-	return ResolveAllGroupMemberships(ctx, db, query.Not(
-		query.Or(
-			query.StringEndsWith(query.StartProperty(common.ObjectID.String()), AdminGroupSuffix),
-			query.StringEndsWith(query.EndProperty(common.ObjectID.String()), AdminGroupSuffix),
-		),
-	))
-}

packages/go/dawgs/ops/traversal.go

@@ -79,7 +79,7 @@ type TraversalPlan struct {
 	Direction             graph.Direction
 	BranchQuery           graph.CriteriaProvider
 	DepthExceptionHandler DepthExceptionHandler
-	expansionFilter       func(segment *graph.PathSegment) bool
+	ExpansionFilter       func(segment *graph.PathSegment) bool
 	DescentFilter         SegmentFilter
 	PathFilter            PathFilter
 	Skip                  int
@@ -169,7 +169,7 @@ func Traversal(tx graph.Transaction, plan TraversalPlan, pathVisitor PathVisitor
 			}
 		}
 
-		if descendents, err := nextTraversal(tx, next, plan.Direction, plan.BranchQuery, requireTraversalOrder, plan.expansionFilter); err != nil {
+		if descendents, err := nextTraversal(tx, next, plan.Direction, plan.BranchQuery, requireTraversalOrder, plan.ExpansionFilter); err != nil {
 			// If the error value is the halt traversal sentinel then don't relay any error upstream
 			if errors.Is(err, ErrHaltTraversal) {
 				break
@@ -241,7 +241,7 @@ func AcyclicTraverseNodes(tx graph.Transaction, plan TraversalPlan, nodeFilter N
 	)
 
 	// Prevent expansion of already-visited nodes
-	plan.expansionFilter = func(segment *graph.PathSegment) bool {
+	plan.ExpansionFilter = func(segment *graph.PathSegment) bool {
 		return visitedBitmap.CheckedAdd(segment.Node.ID.Uint64())
 	}
 
@@ -273,7 +273,7 @@ func AcyclicTraverseTerminals(tx graph.Transaction, plan TraversalPlan) (graph.N
 	)
 
 	// Prevent expansion of already-visited nodes
-	plan.expansionFilter = func(segment *graph.PathSegment) bool {
+	plan.ExpansionFilter = func(segment *graph.PathSegment) bool {
 		return visitedBitmap.CheckedAdd(segment.Node.ID.Uint64())
 	}