Skip to content

Commit e54edae

Browse files
fix: add mutex protection and test skipping for bounty engine
- Added mutex protection for DiscoveredAssets to prevent race conditions during concurrent access - Added RWMutex locks around asset reads/writes in checkpoint save and discovery phase - Implemented test skipping logic to avoid re-running completed tests on resume - Added shouldSkip() checks for each test type (auth, SCIM, API, nmap, nuclei, GraphQL, IDOR) - Updated progress tracking to count skipped tests as completed - Modified execute
1 parent 134f831 commit e54edae

File tree

1 file changed

+38
-11
lines changed

1 file changed

+38
-11
lines changed

internal/orchestrator/bounty_engine.go

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,7 @@ func NewBugBountyEngine(
572572
}
573573

574574
// BugBountyResult contains the complete results of a bug bounty scan
575+
// P0-2 FIX: Add mutex to protect concurrent access to DiscoveredAssets
575576
type BugBountyResult struct {
576577
ScanID string
577578
Target string
@@ -587,6 +588,7 @@ type BugBountyResult struct {
587588
OrganizationInfo *correlation.Organization // Organization footprinting results
588589
DiscoverySession *discovery.DiscoverySession // Asset discovery session metadata
589590
DiscoveredAssets []*discovery.Asset // Discovered assets for display
591+
assetsMutex sync.RWMutex // P0-2: Protects DiscoveredAssets from race conditions
590592
}
591593

592594
// PhaseResult contains results from a specific phase
@@ -669,8 +671,10 @@ func (e *BugBountyEngine) Execute(ctx context.Context, target string) (*BugBount
669671
return
670672
}
671673

672-
// Convert discovered assets for checkpoint serialization
674+
// P0-2 FIX: Lock assets during read to prevent race condition with concurrent modifications
675+
result.assetsMutex.RLock()
673676
checkpointAssets := checkpoint.ConvertDiscoveryAssets(result.DiscoveredAssets)
677+
result.assetsMutex.RUnlock()
674678

675679
// Build checkpoint state
676680
state := &checkpoint.State{
@@ -1131,7 +1135,10 @@ func (e *BugBountyEngine) Execute(ctx context.Context, target string) (*BugBount
11311135
if lastSession != nil {
11321136
result.DiscoverySession = lastSession
11331137
}
1134-
result.DiscoveredAssets = assets // Store assets for display
1138+
// P0-2 FIX: Lock during write to prevent race with checkpoint save
1139+
result.assetsMutex.Lock()
1140+
result.DiscoveredAssets = assets
1141+
result.assetsMutex.Unlock()
11351142

11361143
phaseResult = PhaseResult{
11371144
Phase: "discovery",
@@ -1406,7 +1413,8 @@ func (e *BugBountyEngine) Execute(ctx context.Context, target string) (*BugBount
14061413
}
14071414

14081415
tracker.StartPhase("testing")
1409-
findings, phaseResults := e.executeTestingPhase(ctx, target, prioritized, tracker, dbLogger)
1416+
// Normal run: no tests to skip (empty slice)
1417+
findings, phaseResults := e.executeTestingPhase(ctx, target, prioritized, tracker, dbLogger, []string{})
14101418

14111419
testingDuration := time.Since(testingStart)
14121420
dbLogger.Infow(" Testing phase completed",
@@ -1674,7 +1682,11 @@ discoveryPhase:
16741682
result.DiscoverySession = session
16751683
}
16761684

1685+
// P0-2 FIX: Lock during write to prevent race with checkpoint save
1686+
result.assetsMutex.Lock()
16771687
result.DiscoveredAssets = assets
1688+
result.assetsMutex.Unlock()
1689+
16781690
result.PhaseResults["discovery"] = discoveryPhaseResult
16791691
tracker.CompletePhase("discovery")
16801692

@@ -1691,8 +1703,9 @@ testingPhase:
16911703
completedTests[test] = true
16921704
}
16931705

1706+
// P0-7 FIX: Pass completedTests to skip already-run tests on resume
16941707
// Run testing on all assets (use existing testing phase but skip completed tests)
1695-
newFindings, phaseResults = e.executeTestingPhase(ctx, target, prioritizedAssets, tracker, dbLogger)
1708+
newFindings, phaseResults = e.executeTestingPhase(ctx, target, prioritizedAssets, tracker, dbLogger, state.CompletedTests)
16961709
result.Findings = append(result.Findings, newFindings...)
16971710
for phase, pr := range phaseResults {
16981711
result.PhaseResults[phase] = pr
@@ -2165,7 +2178,7 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
21652178
var wg sync.WaitGroup
21662179

21672180
// Authentication Testing
2168-
if e.config.EnableAuthTesting {
2181+
if e.config.EnableAuthTesting && !shouldSkip("auth") {
21692182
wg.Add(1)
21702183
go func() {
21712184
defer wg.Done()
@@ -2176,10 +2189,12 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
21762189
mu.Unlock()
21772190
updateTestProgress()
21782191
}()
2192+
} else if shouldSkip("auth") {
2193+
updateTestProgress() // Count as completed
21792194
}
21802195

21812196
// SCIM Testing
2182-
if e.config.EnableSCIMTesting {
2197+
if e.config.EnableSCIMTesting && !shouldSkip("scim") {
21832198
wg.Add(1)
21842199
go func() {
21852200
defer wg.Done()
@@ -2190,10 +2205,12 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
21902205
mu.Unlock()
21912206
updateTestProgress()
21922207
}()
2208+
} else if shouldSkip("scim") {
2209+
updateTestProgress()
21932210
}
21942211

21952212
// API Testing
2196-
if e.config.EnableAPITesting {
2213+
if e.config.EnableAPITesting && !shouldSkip("api") {
21972214
wg.Add(1)
21982215
go func() {
21992216
defer wg.Done()
@@ -2204,10 +2221,12 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
22042221
mu.Unlock()
22052222
updateTestProgress()
22062223
}()
2224+
} else if shouldSkip("api") {
2225+
updateTestProgress()
22072226
}
22082227

22092228
// Nmap Service Fingerprinting
2210-
if e.config.EnableServiceFingerprint && e.nmapScanner != nil {
2229+
if e.config.EnableServiceFingerprint && e.nmapScanner != nil && !shouldSkip("nmap") {
22112230
wg.Add(1)
22122231
go func() {
22132232
defer wg.Done()
@@ -2218,10 +2237,12 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
22182237
mu.Unlock()
22192238
updateTestProgress()
22202239
}()
2240+
} else if shouldSkip("nmap") {
2241+
updateTestProgress()
22212242
}
22222243

22232244
// Nuclei Vulnerability Scanning
2224-
if e.config.EnableNucleiScan && e.nucleiScanner != nil {
2245+
if e.config.EnableNucleiScan && e.nucleiScanner != nil && !shouldSkip("nuclei") {
22252246
wg.Add(1)
22262247
go func() {
22272248
defer wg.Done()
@@ -2232,10 +2253,12 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
22322253
mu.Unlock()
22332254
updateTestProgress()
22342255
}()
2256+
} else if shouldSkip("nuclei") {
2257+
updateTestProgress()
22352258
}
22362259

22372260
// GraphQL Testing (Go scanner + optional Python GraphCrawler)
2238-
if e.config.EnableGraphQLTesting && e.graphqlScanner != nil {
2261+
if e.config.EnableGraphQLTesting && e.graphqlScanner != nil && !shouldSkip("graphql") {
22392262
wg.Add(1)
22402263
go func() {
22412264
defer wg.Done()
@@ -2246,10 +2269,12 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
22462269
mu.Unlock()
22472270
updateTestProgress()
22482271
}()
2272+
} else if shouldSkip("graphql") {
2273+
updateTestProgress()
22492274
}
22502275

22512276
// IDOR Testing (Go scanner)
2252-
if e.config.EnableIDORTesting && e.idorScanner != nil {
2277+
if e.config.EnableIDORTesting && e.idorScanner != nil && !shouldSkip("idor") {
22532278
wg.Add(1)
22542279
go func() {
22552280
defer wg.Done()
@@ -2260,6 +2285,8 @@ func (e *BugBountyEngine) executeTestingPhase(ctx context.Context, target string
22602285
mu.Unlock()
22612286
updateTestProgress()
22622287
}()
2288+
} else if shouldSkip("idor") {
2289+
updateTestProgress()
22632290
}
22642291

22652292
// Wait for all tests to complete

0 commit comments

Comments
 (0)