Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: stricter static analysis #355

Merged
merged 7 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .golangci.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
{
"linters": {
"disable": [
"errcheck",
"unused"
"errcheck"
superlinkx marked this conversation as resolved.
Show resolved Hide resolved
],
"enable": [
"gosimple",
Expand All @@ -26,6 +25,10 @@
"path": "foldr_test\\.go",
"text": "SA4000:",
"severity": "warning"
},
{
"path": "(dawgs/util/size/(.+)|dawgs/drivers/pg/statements.go)",
superlinkx marked this conversation as resolved.
Show resolved Hide resolved
"linters": ["unused"]
}
]
},
Expand All @@ -42,7 +45,11 @@
"default-severity": "error",
"rules": [
{
"text": "(ST\\d{4}|S\\d{4}|SA1019)",
"linters": ["stylecheck", "gosimple", "unused", "errcheck", "forcetypeassert"],
superlinkx marked this conversation as resolved.
Show resolved Hide resolved
"severity": "warning"
},
{
"text": "SA1019:",
"severity": "warning"
},
{
Expand Down
9 changes: 2 additions & 7 deletions cmd/api/src/daemons/datapipe/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ package datapipe

import (
"context"
"github.com/specterops/bloodhound/src/database"
"os"

"github.com/specterops/bloodhound/src/database"

"github.com/specterops/bloodhound/dawgs/graph"
"github.com/specterops/bloodhound/log"
"github.com/specterops/bloodhound/src/model"
Expand Down Expand Up @@ -139,9 +140,3 @@ func (s *Daemon) processIngestTasks(ctx context.Context, ingestTasks model.Inges
s.clearFileTask(ingestTask)
}
}

func (s *Daemon) clearTask(ingestTask model.IngestTask) {
if err := s.db.DeleteIngestTask(ingestTask); err != nil {
log.Errorf("Error removing task from db: %v", err)
}
}
superlinkx marked this conversation as resolved.
Show resolved Hide resolved
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
superlinkx marked this conversation as resolved.
Show resolved Hide resolved
}

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
}
superlinkx marked this conversation as resolved.
Show resolved Hide resolved

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
Loading
Loading