[BED-6646] NullReferenceException when using --trackcomputercalls with collection methods that don't enumerate computers #182
Conversation
…s data collected When --trackcomputercalls is used with collection methods that don't enumerate computers (e.g., dconly), CompStatusWriter.FlushWriter() crashes with NullReferenceException because _streamWriter is never initialized. CreateFile() is only called when the first status object is accepted via the BaseWriter pattern, but collection methods like dconly don't generate any computer status messages, leaving _streamWriter as null. Added null check in FlushWriter() to gracefully handle the no-data case.
WalkthroughAdded an early-return guard in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Writers/CompStatusWriter.cs (1)
22-23: Remove duplicate assignment.The assignment
_channel = channel;is duplicated on consecutive lines. While harmless, this should be cleaned up.Apply this diff:
_context = context; _channel = channel; - _channel = channel; if (!_context.Flags.DumpComputerStatus) NoOp = true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Writers/CompStatusWriter.cs(1 hunks)
🔇 Additional comments (1)
src/Writers/CompStatusWriter.cs (1)
48-49: LGTM! Null guard correctly prevents the crash.The early return when
_streamWriteris null is the right fix for the edge case whereCreateFile()is never called (no computers enumerated). This prevents NullReferenceExceptions inWriteData()(line 29),FlushAsync()(line 51), andCloseLog()(line 71).
Changed from checking null to checking !FileCreated to match the pattern used by other writers like JsonDataWriter
Description
When
--trackcomputercallsis used with collection methods that don't perform computer enumeration (e.g.,dconly,group, `trusts´), SharpHound crashes during end of collection. JSONs and ZIP is still produced.CompStatusWriter crashes with a NullReferenceException during shutdown. The _streamWriter field remains null because CreateFile() is only called when the first status object is accepted, but no status objects are generated when computers aren't enumerated.
Output from crash:
Motivation and Context
https://specterops.atlassian.net/browse/BED-6646
How Has This Been Tested?
Output from fix:
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit