SpecterOps / SpecterOps/SharpHoundCommon

Null exception

Open
#44 0 comments 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.