Skip to content

Commit

Permalink
fix: linter fixes, primarily targeting a function that didn't need to…
Browse files Browse the repository at this point in the history
… return an error
  • Loading branch information
superlinkx committed Jan 31, 2024
1 parent 322dca4 commit dada015
Show file tree
Hide file tree
Showing 15 changed files with 92 additions and 346 deletions.
7 changes: 3 additions & 4 deletions .golangci.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
],
"enable": [
"gosimple",
"stylecheck",
"forcetypeassert"
"stylecheck"
]
},
"issues": {
Expand Down Expand Up @@ -46,11 +45,11 @@
"default-severity": "error",
"rules": [
{
"linters": ["unused"],
"linters": ["stylecheck", "gosimple", "unused", "errcheck", "forcetypeassert"],
"severity": "warning"
},
{
"text": "(ST\\d{4}|S\\d{4}|SA1019)",
"text": "SA1019:",
"severity": "warning"
},
{
Expand Down
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ prune-my-branches nuclear='no':
# Run all analyzers (requires jq to be installed locally)
analyze:
go run github.com/specterops/bloodhound/packages/go/stbernard analysis | jq 'sort_by(.severity) | .[] | {"severity": .severity, "description": .description, "location": "\(.location.path):\(.location.lines.begin)"}'
go run github.com/specterops/bloodhound/packages/go/stbernard analysis | jq 'sort_by(.severity) | .[] | {"severity": .severity, "description": .description, "location": "\(.location.path):\(.location.lines.begin)"}'

# run docker compose commands for the BH dev profile (Default: up)
bh-dev *ARGS='up':
Expand Down
59 changes: 30 additions & 29 deletions packages/go/analysis/ad/ad.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,14 @@ func CalculateCrossProductNodeSets(groupExpansions impact.PathAggregator, nodeSe
//Find all the groups in our secondary targets and map them to their cardinality in our expansions
//Saving off to a map to prevent multiple lookups on the expansions
//Unhandled error here is irrelevant, we can never return an error
unrollSet.Each(func(id uint32) (bool, error) {
unrollSet.Each(func(id uint32) bool {
//If group expansions contains this ID and its cardinality is > 0, it's a group/localgroup
idCardinality := groupExpansions.Cardinality(id).Cardinality()
if idCardinality > 0 {
tempMap[id] = idCardinality
}

return true, nil
return true
})

//Save the map keys to a new slice, this represents our list of groups in the expansion
Expand Down Expand Up @@ -391,12 +391,12 @@ func CalculateCrossProductNodeSets(groupExpansions impact.PathAggregator, nodeSe
}
}

unrollSet.Each(func(remainder uint32) (bool, error) {
unrollSet.Each(func(remainder uint32) bool {
if checkSet.Contains(remainder) {
resultEntities.Add(remainder)
}

return true, nil
return true
})

return resultEntities
Expand All @@ -417,7 +417,7 @@ func CalculateCrossProductBitmaps(groupExpansions impact.PathAggregator, nodeSet
)

//Take the second of our node sets and unroll it all into a single bitmap
nodeSets[1].Each(func(id uint32) (bool, error) {
nodeSets[1].Each(func(id uint32) bool {
checkSet.Add(id)
idCardinality := groupExpansions.Cardinality(id)
idCardinalityCount := getCardinalityCount(id, idCardinality, cardinalityCache)
Expand All @@ -426,14 +426,14 @@ func CalculateCrossProductBitmaps(groupExpansions impact.PathAggregator, nodeSet
checkSet.Or(idCardinality.(cardinality.Duplex[uint32]))
}

return true, nil
return true
})

//If we have more than 2 bitmaps, we need to AND everything together
if len(nodeSets) > 2 {
for i := 2; i < len(nodeSets); i++ {
tempSet := cardinality.NewBitmap32()
nodeSets[i].Each(func(id uint32) (bool, error) {
nodeSets[i].Each(func(id uint32) bool {
tempSet.Add(id)
idCardinality := groupExpansions.Cardinality(id)
idCardinalityCount := getCardinalityCount(id, idCardinality, cardinalityCache)
Expand All @@ -442,7 +442,7 @@ func CalculateCrossProductBitmaps(groupExpansions impact.PathAggregator, nodeSet
tempSet.Or(idCardinality.(cardinality.Duplex[uint32]))
}

return true, nil
return true
})

checkSet.And(tempSet)
Expand All @@ -451,7 +451,7 @@ func CalculateCrossProductBitmaps(groupExpansions impact.PathAggregator, nodeSet

//checkSet should have all the valid principals from all other sets at this point
//Check first degree principals in our reference set first
nodeSets[0].Each(func(id uint32) (bool, error) {
nodeSets[0].Each(func(id uint32) bool {
if checkSet.Contains(id) {
resultEntities.Add(id)
} else {
Expand All @@ -463,21 +463,21 @@ func CalculateCrossProductBitmaps(groupExpansions impact.PathAggregator, nodeSet
}
}

return true, nil
return true
})

tempMap := map[uint32]uint64{}
//Find all the groups in our secondary targets and map them to their cardinality in our expansions
//Saving off to a map to prevent multiple lookups on the expansions
//Unhandled error here is irrelevant, we can never return an error
unrollSet.Each(func(id uint32) (bool, error) {
unrollSet.Each(func(id uint32) bool {
//If group expansions contains this ID and its cardinality is > 0, it's a group/localgroup
idCardinality := groupExpansions.Cardinality(id).Cardinality()
if idCardinality > 0 {
tempMap[id] = idCardinality
}

return true, nil
return true
})

//Save the map keys to a new slice, this represents our list of groups in the expansion
Expand Down Expand Up @@ -509,12 +509,12 @@ func CalculateCrossProductBitmaps(groupExpansions impact.PathAggregator, nodeSet
}
}

unrollSet.Each(func(remainder uint32) (bool, error) {
unrollSet.Each(func(remainder uint32) bool {
if checkSet.Contains(remainder) {
resultEntities.Add(remainder)
}

return true, nil
return true
})

return resultEntities
Expand Down Expand Up @@ -746,6 +746,7 @@ func ADCSESC6aPath4Pattern(domainId graph.ID, enterpriseCAs cardinality.Duplex[u

func GetADCSESC6aEdgeComposition(ctx context.Context, db graph.Database, edge *graph.Relationship) (graph.PathSet, error) {
var (
closureErr error
startNode *graph.Node
traversalInst = traversal.New(db, analysis.MaximumDatabaseParallelWorkers)
lock = &sync.Mutex{}
Expand Down Expand Up @@ -844,8 +845,7 @@ func GetADCSESC6aEdgeComposition(ctx context.Context, db graph.Database, edge *g
log.Warnf("unable to access property %s for node with id %d: %v", common.Email.String(), startNode.ID, err)
}

if err := certTemplates.Each(func(value uint32) (bool, error) {

certTemplates.Each(func(value uint32) bool {
var certTemplate *graph.Node

if err := db.ReadTransaction(ctx, func(tx graph.Transaction) error {
Expand All @@ -856,7 +856,8 @@ func GetADCSESC6aEdgeComposition(ctx context.Context, db graph.Database, edge *g
return nil
}
}); err != nil {
return false, err
closureErr = fmt.Errorf("could not fetch cert template node: %w", err)
return false
}

schemaVersion, err := certTemplate.Properties.Get(ad.SchemaVersion.String()).Float64()
Expand All @@ -881,7 +882,6 @@ func GetADCSESC6aEdgeComposition(ctx context.Context, db graph.Database, edge *g
}

for _, segment := range certTemplateSegments[graph.ID(value)] {

if startNode.Kinds.ContainsOneOf(ad.User) {
if subjectAltRequireDNS || subjectAltRequireDomainDNS {
continue
Expand All @@ -905,21 +905,20 @@ func GetADCSESC6aEdgeComposition(ctx context.Context, db graph.Database, edge *g

}

return true, nil
}); err != nil {
return paths, err
return true
})

if closureErr != nil {
return paths, closureErr
}

if paths.Len() > 0 {
if err := enterpriseCAs.Each(func(value uint32) (bool, error) {
enterpriseCAs.Each(func(value uint32) bool {
for _, segment := range enterpriseCASegments[graph.ID(value)] {
paths.AddPath(segment.Path())
}
return true, nil

}); err != nil {
return paths, err
}
return true
})
}

return paths, nil
Expand Down Expand Up @@ -1226,13 +1225,15 @@ func GetADCSESC1EdgeComposition(ctx context.Context, db graph.Database, edge *gr
path1EnterpriseCAs.And(path2EnterpriseCAs)

// Render paths from the segments
return paths, path1EnterpriseCAs.Each(func(value uint32) (bool, error) {
path1EnterpriseCAs.Each(func(value uint32) bool {
for _, segment := range candidateSegments[graph.ID(value)] {
paths.AddPath(segment.Path())
}

return true, nil
return true
})

return paths, nil
}

func getGoldenCertEdgeComposition(tx graph.Transaction, edge *graph.Relationship) (graph.PathSet, error) {
Expand Down
22 changes: 10 additions & 12 deletions packages/go/analysis/ad/adcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ func PostADCSESC1(ctx context.Context, tx graph.Transaction, outC chan<- analysi
}
}

results.Each(func(value uint32) (bool, error) {
results.Each(func(value uint32) bool {
if !channels.Submit(ctx, outC, analysis.CreatePostRelationshipJob{
FromID: graph.ID(value),
ToID: domain.ID,
Kind: ad.ADCSESC1,
}) {
return false, nil
return false
} else {
return true, nil
return true
}
})

Expand Down Expand Up @@ -170,15 +170,15 @@ func PostADCSESC3(ctx context.Context, tx graph.Transaction, outC chan<- analysi
}
}

results.Each(func(value uint32) (bool, error) {
results.Each(func(value uint32) bool {
if !channels.Submit(ctx, outC, analysis.CreatePostRelationshipJob{
FromID: graph.ID(value),
ToID: domain.ID,
Kind: ad.ADCSESC3,
}) {
return false, nil
return false
} else {
return true, nil
return true
}
})

Expand Down Expand Up @@ -329,19 +329,17 @@ func PostADCSESC6a(ctx context.Context, tx graph.Transaction, outC chan<- analys
}
}

if err := filterTempResultsForESC6(tx, tempResults, groupExpansions, validCertTemplates, cache).Each(func(value uint32) (bool, error) {
filterTempResultsForESC6(tx, tempResults, groupExpansions, validCertTemplates, cache).Each(func(value uint32) bool {
if !channels.Submit(ctx, outC, analysis.CreatePostRelationshipJob{
FromID: graph.ID(value),
ToID: domain.ID,
Kind: ad.ADCSESC6a,
}) {
return false, nil
return false
} else {
return true, nil
return true
}
}); err != nil {
return err
}
})
}
return nil
}
Expand Down
7 changes: 4 additions & 3 deletions packages/go/analysis/ad/esc10.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package ad
import (
"context"
"errors"

"github.com/specterops/bloodhound/analysis"
"github.com/specterops/bloodhound/analysis/impact"
"github.com/specterops/bloodhound/dawgs/cardinality"
Expand Down Expand Up @@ -103,15 +104,15 @@ func PostADCSESC10a(ctx context.Context, tx graph.Transaction, outC chan<- analy
}
}

results.Each(func(value uint32) (bool, error) {
results.Each(func(value uint32) bool {
if !channels.Submit(ctx, outC, analysis.CreatePostRelationshipJob{
FromID: graph.ID(value),
ToID: domain.ID,
Kind: ad.ADCSESC10a,
}) {
return false, nil
return false
} else {
return true, nil
return true
}
})

Expand Down
18 changes: 8 additions & 10 deletions packages/go/analysis/ad/esc6.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,20 +203,18 @@ func PostADCSESC6b(ctx context.Context, tx graph.Transaction, outC chan<- analys
}
}

if err := filterTempResultsForESC6(tx, tempResults, groupExpansions, validCertTemplates, cache).Each(
func(value uint32) (bool, error) {
filterTempResultsForESC6(tx, tempResults, groupExpansions, validCertTemplates, cache).Each(
func(value uint32) bool {
if !channels.Submit(ctx, outC, analysis.CreatePostRelationshipJob{
FromID: graph.ID(value),
ToID: domain.ID,
Kind: ad.ADCSESC6b,
}) {
return false, nil
return false
} else {
return true, nil
return true
}
}); err != nil {
return err
}
})
}
return nil
}
Expand All @@ -238,11 +236,11 @@ func isCertTemplateValidForEsc6b(reqManagerApproval, authenticationEnabled bool,
func filterTempResultsForESC6(tx graph.Transaction, tempResults cardinality.Duplex[uint32], groupExpansions impact.PathAggregator, validCertTemplates []*graph.Node, cache ADCSCache) cardinality.Duplex[uint32] {
principalsEnabledForESC6 := cardinality.NewBitmap32()

tempResults.Each(func(value uint32) (bool, error) {
tempResults.Each(func(value uint32) bool {
sourceID := graph.ID(value)

if resultNode, err := tx.Nodes().Filter(query.Equals(query.NodeID(), sourceID)).First(); err != nil {
return true, nil
return true
} else {
if resultNode.Kinds.ContainsOneOf(ad.Group) {
//A Group will be added to the list since it requires no further conditions
Expand All @@ -259,7 +257,7 @@ func filterTempResultsForESC6(tx graph.Transaction, tempResults cardinality.Dupl
}
}
}
return true, nil
return true
})
return principalsEnabledForESC6
}
Loading

0 comments on commit dada015

Please sign in to comment.