search

netwrix / pingcastle

PingCastle - Get Active Directory Security at 80% in 20% of the time
https://www.pingcastle.com
Other
2.35k stars 292 forks source link

fixed bug in T-SIDHistoryDangerous finding detection #158

Closed imaibou closed 1 year ago

imaibou commented 1 year ago

As it stands, this rule is never triggered because of a small bug. The "count" variable that is returned is never incremented, so the check always returns 0. This PR fixes it.

vletoux commented 1 year ago

indeed, thanks for submitting. I remind that PingCastle is done on a private repo then pushed every 6 months so integrating commits from this branch is sometimes difficult.

In this case, this is a trace of early rule where a count was returned. Now most of the rules relies on AddRawDetail where the count is implicitely done. The function just add to return null instead of count.

I made a commit on my side to fix the issue. image

Thanks for reporting

imaibou commented 1 year ago

Great !

Noted. I will open issues in the future instead of committing PRs to make it easier :)

Thank you for the great tool.