BC-SECURITY / Moriarty

Moriarty is designed to enumerate missing KBs, detect various vulnerabilities, and suggest potential exploits for Privilege Escalation in Windows environments.
GNU General Public License v3.0
431 stars 54 forks source link

Evaluation of PrinteSpooler Checks #2

Closed LuemmelSec closed 2 months ago

LuemmelSec commented 4 months ago

In your code you always set it to vulnerable, no matter what the outcome of the checks is:

CheckSpoolerService(); CheckPatchStatus(); CheckRegistrySettings();

vulnerabilities.SetAsVulnerable(Id);

Hence, it always shows vulnerable.

It would be better to place the setting into each of your functions and then ONLY if the checks implies that the system IS vulnerable. For example here:

switch (sc.Status) { case ServiceControllerStatus.Running: DebugUtility.DebugPrint("Print Spooler service is ENABLED and RUNNING."); SET VULNERABLE break; case ServiceControllerStatus.Stopped: DebugUtility.DebugPrint("Print Spooler service is ENABLED but STOPPED."); break; default: DebugUtility.DebugPrint("Print Spooler service status is UNKNOWN."); break; }

It only makes sense to set vulnerable when it is enabled and running, maybe when stopped. It would also make sense to have a "might be vulnerable" option as you also check for those options. Just saying it IS vulnerable is pretty misleading in the default output.

Cx01N commented 4 months ago

Good catch, I must have missed that. I appreciate it.

Now that I am looking at the code I based it off of, I see that I should reorganize it a bit more. I moved the test onto a system that I know isn't vulnerable and is still coming back vulnerable. https://github.com/xbufu/PrintNightmareCheck/blob/main/Invoke-NightmareCheck.ps1

LuemmelSec commented 4 months ago

Sounds like a plan :) you are welcome.

Cx01N commented 4 months ago

I made the updates if you want to check it out again.

LuemmelSec commented 4 months ago

It looks good now :)