SpecterOps / SpecterOps/SharpHoundCommon
Null exception
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 104
- Forks
- 56
- Avg merge
- 3d 23h
- Merged PRs (30d)
- 2
Description
This exception occurred during DCOnly collection:
2022-11-21T13:50:51.0367367-05:00|INFORMATION|LDAP channel closed, waiting for consumers
2022-11-21T13:50:52.8803968-05:00|ERROR|error in consumer
System.NullReferenceException: Object reference not set to an instance of an object.
at SharpHoundCommonLib.Processors.GPOLocalGroupProcessor.<ProcessGPOXmlFile>d__13.MoveNext()
at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
at SharpHoundCommonLib.Processors.GPOLocalGroupProcessor.<ReadGPOLocalGroups>d__10.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Sharphound.Runtime.ObjectProcessors.<ProcessDomainObject>d__20.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Sharphound.Runtime.ObjectProcessors.<ProcessObject>d__15.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Sharphound.Runtime.LDAPConsumer.<ConsumeSearchResults>d__0.MoveNext()
2022-11-21T13:50:52.9272690-05:00|INFORMATION|Consumers finished, closing output channel
2022-11-21T13:50:53.0366411-05:00|INFORMATION|Output channel closed, waiting for output task to complete
Closing writers
2022-11-21T13:50:53.1460060-05:00|INFORMATION|Status: 42918 objects finished (+6523 232.5313)/s -- Using 448 MB RAM
2022-11-21T13:50:53.1460060-05:00|INFORMATION|Enumeration finished in 00:03:12.3442529
2022-11-21T13:50:56.4988165-05:00|INFORMATION|SharpHound Enumeration Completed at 1:50 PM on 11/21/2022! Happy Graphing!
There error is from here. Nowhere else is ToList() called on ProcessGPOXmlFile objects in that method.
Should probably add some null check for all the ToList() calls in that method, and examine other uses of generators/ToList throughout the codebase for potential problem areas.
ProcessGPOXmlFile is a generator function (returns values via yield). If generator functions never return a value (or break early returning nothing), then null is returned. So you either need to check for null when using generators that might not return something, or ensure all generator functions return something. While initially difficult to do, enabling (nullable types)[https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references] (be it in individual files or project-wide) can help eliminate this bug class throughout the entire codebase (nullable types is enabled by default in new .NET projects).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/CommonLib/Processors/GPOLocalGroupProcessor.cs around line 141, where ProcessGPOXmlFile is materialized during DCOnly collection. Trace the generator and the other ToList calls in that method, then verify that a DCOnly collection completes without the reported NullReferenceException. The broader nullable-reference and generator audit should be treated as additional scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100