diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cd3c825..06de0d4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -54,7 +54,7 @@ jobs: timeout-minutes: 20 services: neo4j: - image: neo4j:5-community + image: neo4j:4.4 env: NEO4J_AUTH: neo4j/testpassword NEO4J_PLUGINS: '[]' diff --git a/README.md b/README.md index 3dfa7c7..411a07d 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ ADPathfinder is an attack mapping tool for pentesters and red teamers. It analys

- ADPathfinder demo video + ADPathfinder CLI demo showing grouped risk findings and attack paths

@@ -132,7 +132,7 @@ adpathfinder --ad --pwd Contoso,ContosoIT --ntds ntds.txt -p hashcat.potfile --d

- AD HTML report demo video + ADPathFinder AD report walkthrough showing overview, mitigation, and confirmation steps

@@ -140,7 +140,7 @@ adpathfinder --ad --pwd Contoso,ContosoIT --ntds ntds.txt -p hashcat.potfile --d

- Password audit HTML report demo video + ADPathFinder password audit report walkthrough showing cracked categories, policy compliance, password lengths, and roastable accounts

diff --git a/checks/bad_successor.py b/checks/bad_successor.py index ecdfc16..9aa3506 100644 --- a/checks/bad_successor.py +++ b/checks/bad_successor.py @@ -1,66 +1,19 @@ from checks.core import Check, check -@check(risk="High", category="BadSuccessor - Default Groups with OU Privileges", +@check(risk="High", category="BadSuccessor - Privileged OU/Container Control", entity="user", data=["bad_successor_ou_privileges"]) class BadSuccessorCheck(Check): - - CRITICAL_GROUPS_BY_SID = { - 'S-1-5-11': 'AUTHENTICATED USERS', - '-513': 'DOMAIN USERS', - '-515': 'DOMAIN COMPUTERS', - 'S-1-1-0': 'EVERYONE', - 'S-1-5-32-545': 'USERS', - } - DANGEROUS_PERMISSIONS = { - 'GenericAll', 'WriteOwner', 'WriteDacl', 'Owns', - 'GenericWrite', 'WriteOwnerRaw', 'OwnsRaw', - 'WriteGPLink', 'GPLink', 'CreateChild', - } - def execute(self): - return self.check_critical_groups_ou_privileges() - - def check_critical_groups_ou_privileges(self): results = {} - - try: - ou_privileges = self.get_bad_successor_ou_privileges() - - for record in ou_privileges: - if not record: - continue - - entity_sid = record.get('entity_sid', '') - rel_type = record.get('relationship_type', '') - target_name = record.get('target_name', record.get('ou_name', '')) - - for sid_pattern, group_display in self.CRITICAL_GROUPS_BY_SID.items(): - if entity_sid and self._matches_sid_pattern(entity_sid, sid_pattern): - if rel_type in self.DANGEROUS_PERMISSIONS: - description = f"{group_display} HAS {rel_type.upper()} ON OU: {target_name}" - results[description] = self.finding("", inline=True) - break # Only match one group per entity - - except Exception: - pass - + for record in self.get_bad_successor_ou_privileges(): + if not record: + continue + entity_name = record.get('entity_name') or record.get('entity_id') or 'Unknown' + rel_type = record.get('relationship_type') or '' + target_type = record.get('target_type', 'OU') + target_name = record.get('target_name') or record.get('target_id') or 'Unknown' + description = f"{entity_name} HAS {rel_type.upper()} ON {target_type.upper()}: {target_name}" + results[description] = self.finding("", inline=True) return results - - def _matches_sid_pattern(self, entity_sid, pattern): - if not entity_sid or not pattern: - return False - - if pattern.startswith('S-1-'): - return entity_sid.endswith(pattern) - elif pattern.startswith('-'): - if not entity_sid.endswith(pattern): - return False - prefix_pos = len(entity_sid) - len(pattern) - if prefix_pos <= 0: - return False - preceding_char = entity_sid[prefix_pos - 1] - return preceding_char.isdigit() or preceding_char == '-' - else: - return pattern in entity_sid \ No newline at end of file diff --git a/checks/core/mssql_common.py b/checks/core/mssql_common.py index 04cbea8..3ea1ea2 100644 --- a/checks/core/mssql_common.py +++ b/checks/core/mssql_common.py @@ -68,10 +68,10 @@ def linked_server_target_resolution(real_var, stub_var): ) -def host_sid_resolves_to_single_server(sid_expr): - # Avoid bridging ambiguous host SID prefixes shared by sibling instances. +def host_sid_resolves_to_single_server(sid_expr, source_var): return ( - f"COUNT {{ MATCH (sidServer:MSSQL_Server) " - f"WHERE sidServer.objectid CONTAINS ':' " - f"AND split(sidServer.objectid, ':')[0] = {sid_expr} }} = 1" + f"NOT EXISTS {{ MATCH (sidDup:MSSQL_Server) " + f"WHERE sidDup <> {source_var} " + f"AND sidDup.objectid CONTAINS ':' " + f"AND split(sidDup.objectid, ':')[0] = {sid_expr} }}" ) diff --git a/checks/mssql_privilege_escalation.py b/checks/mssql_privilege_escalation.py index 5aa48df..3ce71ee 100644 --- a/checks/mssql_privilege_escalation.py +++ b/checks/mssql_privilege_escalation.py @@ -292,7 +292,7 @@ def _fetch_linked_server_paths(self): OR (stub.objectid IS NOT NULL AND srvA.objectid IS NOT NULL AND stub.objectid CONTAINS ':' AND srvA.objectid CONTAINS ':' AND split(stub.objectid, ':')[0] = split(srvA.objectid, ':')[0] - AND {host_sid_resolves_to_single_server("split(srvA.objectid, ':')[0]")}) + AND {host_sid_resolves_to_single_server("split(srvA.objectid, ':')[0]", "srvA")}) MATCH (srvB:MSSQL_Server)-[:MSSQL_Contains]->(target:MSSQL_ServerRole) WHERE {linked_server_target_resolution('srvB', 'srvB_stub')} diff --git a/checks/sccm_elevate1.py b/checks/sccm_elevate1.py index 0397f4f..0c563c5 100644 --- a/checks/sccm_elevate1.py +++ b/checks/sccm_elevate1.py @@ -22,6 +22,7 @@ def execute(self): rows = self.query(f""" MATCH (g)-[:CoerceAndRelayToSMB]->(target:Computer) WHERE target.SMBSigningRequired = false + AND coalesce(target.enabled, true) = true AND target.SCCMSiteSystemRoles IS NOT NULL {cdf} UNWIND target.SCCMSiteSystemRoles as role @@ -30,6 +31,7 @@ def execute(self): OPTIONAL MATCH (ss:Computer) WHERE ss.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(ss.enabled, true) = true AND any(r IN ss.SCCMSiteSystemRoles WHERE r = 'SMS Site Server@' + site_code) AND ss.objectid <> target.objectid diff --git a/checks/sccm_privilege_escalation.py b/checks/sccm_privilege_escalation.py index 4f0b95d..39e6ae0 100644 --- a/checks/sccm_privilege_escalation.py +++ b/checks/sccm_privilege_escalation.py @@ -380,7 +380,7 @@ def _fetch_linked_sccm_paths(self): OR (stub.objectid IS NOT NULL AND srvA.objectid IS NOT NULL AND stub.objectid CONTAINS ':' AND srvA.objectid CONTAINS ':' AND split(stub.objectid, ':')[0] = split(srvA.objectid, ':')[0] - AND {host_sid_resolves_to_single_server("split(srvA.objectid, ':')[0]")}) + AND {host_sid_resolves_to_single_server("split(srvA.objectid, ':')[0]", "srvA")}) MATCH (srvB:MSSQL_Server) WHERE {linked_server_target_resolution('srvB', 'srvB_stub')} @@ -506,7 +506,7 @@ def _fetch_linked_execute_host_sccm_paths(self): AND stub.objectid CONTAINS ':' AND src.objectid CONTAINS ':' AND split(stub.objectid, ':')[0] = split(src.objectid, ':')[0] - AND {host_sid_resolves_to_single_server("split(src.objectid, ':')[0]")} + AND {host_sid_resolves_to_single_server("split(src.objectid, ':')[0]", "src")} MATCH (linked:MSSQL_Server) WHERE linked = linkedStub OR {linked_server_target_resolution('linked', 'linkedStub')} diff --git a/checks/sccm_takeover1.py b/checks/sccm_takeover1.py index b9ef369..a706c11 100644 --- a/checks/sccm_takeover1.py +++ b/checks/sccm_takeover1.py @@ -23,17 +23,24 @@ class SCCMTakeover1Check(SCCMDomainMixin, Check): ) def execute(self): + source_domain_cond = self._domain_condition("g") sdf = self._site_domain_condition() rows = self.query(f""" MATCH (g)-[relay:CoerceAndRelayToMSSQL]->(login) -[:MSSQL_MemberOf|MSSQL_IsMappedTo|MSSQL_ControlServer |MSSQL_ControlDB|MSSQL_Contains |SCCM_AssignAllPermissions*1..6]->(site:SCCM_Site) - WHERE true {sdf} + WHERE ( + ((g:User OR g:Computer OR g:Group){source_domain_cond}) + OR NOT (g:User OR g:Computer OR g:Group) + ) + AND (NOT g:Computer OR coalesce(g.enabled, true) = true) + AND true{sdf} OPTIONAL MATCH (login)-[:MSSQL_MemberOf]->(:MSSQL_ServerRole) -[:MSSQL_ControlServer]->(server:MSSQL_Server) OPTIONAL MATCH (target:Computer)-[:MSSQL_HostFor]->(server) + WHERE coalesce(target.enabled, true) = true RETURN DISTINCT site.siteCode as site_code, diff --git a/checks/sccm_takeover2.py b/checks/sccm_takeover2.py index 400631b..671f54f 100644 --- a/checks/sccm_takeover2.py +++ b/checks/sccm_takeover2.py @@ -27,10 +27,12 @@ def execute(self): MATCH (g)-[:CoerceAndRelayToSMB]->(target:Computer) -[:MSSQL_HostFor]->(:MSSQL_Server)-[:MSSQL_Contains]->(db:MSSQL_Database) -[:SCCM_AssignAllPermissions]->(site:SCCM_Site) - WHERE target.SMBSigningRequired = false {cdf} + WHERE target.SMBSigningRequired = false + AND coalesce(target.enabled, true) = true {cdf} OPTIONAL MATCH (ss:Computer) WHERE ss.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(ss.enabled, true) = true AND any(role IN ss.SCCMSiteSystemRoles WHERE role = 'SMS Site Server@' + site.siteCode) AND ss.objectid <> target.objectid diff --git a/checks/sccm_takeover4.py b/checks/sccm_takeover4.py index bc6274e..eb852f0 100644 --- a/checks/sccm_takeover4.py +++ b/checks/sccm_takeover4.py @@ -44,6 +44,7 @@ def _fetch_paths(self): OPTIONAL MATCH (cas_comp:Computer) WHERE cas_comp.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(cas_comp.enabled, true) = true AND any(role IN cas_comp.SCCMSiteSystemRoles WHERE role = 'SMS Site Server@' + parent_site.siteCode) WITH parent_site, child_site, @@ -51,6 +52,7 @@ def _fetch_paths(self): MATCH (child_comp:Computer) WHERE child_comp.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(child_comp.enabled, true) = true AND any(role IN child_comp.SCCMSiteSystemRoles WHERE role = 'SMS Site Server@' + child_site.siteCode) AND child_comp.SMBSigningRequired = false diff --git a/checks/sccm_takeover5.py b/checks/sccm_takeover5.py index fcd3329..7b8283c 100644 --- a/checks/sccm_takeover5.py +++ b/checks/sccm_takeover5.py @@ -41,6 +41,7 @@ def _fetch_paths(self): MATCH (ss_comp:Computer) WHERE ss_comp.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(ss_comp.enabled, true) = true AND any(role IN ss_comp.SCCMSiteSystemRoles WHERE role = 'SMS Site Server@' + site.siteCode) WITH site, @@ -49,6 +50,7 @@ def _fetch_paths(self): MATCH (sp_comp:Computer) WHERE sp_comp.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(sp_comp.enabled, true) = true AND any(role IN sp_comp.SCCMSiteSystemRoles WHERE role = 'SMS Provider@' + site.siteCode) WITH site, site_server_sids, site_servers, diff --git a/checks/sccm_takeover6.py b/checks/sccm_takeover6.py index 6d2fc09..2d41bdd 100644 --- a/checks/sccm_takeover6.py +++ b/checks/sccm_takeover6.py @@ -21,11 +21,13 @@ def execute(self): rows = self.query(f""" MATCH (g)-[:CoerceAndRelayToSMB]->(target:Computer)-[:SCCM_AssignAllPermissions]->(site:SCCM_Site) - WHERE target.SMBSigningRequired = false {cdf} + WHERE target.SMBSigningRequired = false + AND coalesce(target.enabled, true) = true {cdf} AND any(role IN target.SCCMSiteSystemRoles WHERE role = 'SMS Provider@' + site.siteCode) OPTIONAL MATCH (ss:Computer) WHERE ss.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(ss.enabled, true) = true AND any(role IN ss.SCCMSiteSystemRoles WHERE role = 'SMS Site Server@' + site.siteCode) AND ss.objectid <> target.objectid diff --git a/checks/sccm_takeover7.py b/checks/sccm_takeover7.py index 8fad8d6..e2c740f 100644 --- a/checks/sccm_takeover7.py +++ b/checks/sccm_takeover7.py @@ -41,7 +41,8 @@ def _fetch_paths(self): rows = self.query(f""" MATCH (role_comp:Computer) - WHERE role_comp.SCCMSiteSystemRoles IS NOT NULL{comp_filter} + WHERE role_comp.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(role_comp.enabled, true) = true{comp_filter} UNWIND role_comp.SCCMSiteSystemRoles as role WITH role_comp, trim(split(role, '@')[0]) as role_name, diff --git a/checks/sccm_takeover8.py b/checks/sccm_takeover8.py index ce8a66d..cd873e0 100644 --- a/checks/sccm_takeover8.py +++ b/checks/sccm_takeover8.py @@ -42,6 +42,7 @@ def execute(self): WHERE ((dc.ldapavailable = true AND dc.ldapsigning = false) OR (dc.ldapsavailable = true AND dc.ldapsepa = false)) + AND coalesce(dc.enabled, true) = true {cdf} WITH dc, (dc.ldapavailable = true AND dc.ldapsigning = false) AS ldap_relay, @@ -51,6 +52,7 @@ def execute(self): WHERE coalesce(src.webclientrunning, src.WebClientRunning, false) = true AND src.domain = dc.domain AND src.SCCMSiteSystemRoles IS NOT NULL + AND coalesce(src.enabled, true) = true AND ( any(role IN src.SCCMSiteSystemRoles WHERE role STARTS WITH 'SMS Site Server@') OR any(role IN src.SCCMSiteSystemRoles WHERE role STARTS WITH 'SMS Provider@') diff --git a/checks/tier0_session_exposure.py b/checks/tier0_session_exposure.py index 5591110..2ff17a0 100644 --- a/checks/tier0_session_exposure.py +++ b/checks/tier0_session_exposure.py @@ -11,26 +11,23 @@ def execute(self): if not self.neo4j_data: return {} - try: - rows = self.neo4j_data.conn.query(""" - MATCH (c:Computer)-[:HasSession]->(u:User) - WHERE u.system_tags CONTAINS 'admin_tier_0' - AND (c.system_tags IS NULL OR NOT c.system_tags CONTAINS 'admin_tier_0') - AND u.enabled = true - AND c.enabled = true - RETURN u.objectid AS sid, - collect(DISTINCT c.name) AS hosts - """, name="tier0_session_exposure") + rows = self.neo4j_data.conn.query(""" + MATCH (c:Computer)-[:HasSession]->(u:User) + WHERE u.system_tags CONTAINS 'admin_tier_0' + AND (c.system_tags IS NULL OR NOT c.system_tags CONTAINS 'admin_tier_0') + AND u.enabled = true + AND c.enabled = true + RETURN u.objectid AS sid, + collect(DISTINCT c.name) AS hosts + """, name="tier0_session_exposure") - findings = {} - for row in rows: - sid = row.get('sid') - hosts = sorted(row.get('hosts') or []) - if sid and hosts: - findings[sid] = self.finding( - f"Active session on: {', '.join(hosts)}", - details={"hosts": hosts}, - ) - return findings - except Exception: - return {} + findings = {} + for row in rows: + sid = row.get('sid') + hosts = sorted(row.get('hosts') or []) + if sid and hosts: + findings[sid] = self.finding( + f"Active session on: {', '.join(hosts)}", + details={"hosts": hosts}, + ) + return findings diff --git a/docs/images/ad-report-slideshow.mp4 b/docs/images/ad-report-slideshow.mp4 index e53927e..a15272a 100644 Binary files a/docs/images/ad-report-slideshow.mp4 and b/docs/images/ad-report-slideshow.mp4 differ diff --git a/docs/images/ad-report-slideshow.webp b/docs/images/ad-report-slideshow.webp index 0877448..6edef44 100644 Binary files a/docs/images/ad-report-slideshow.webp and b/docs/images/ad-report-slideshow.webp differ diff --git a/docs/images/demo-slideshow.mp4 b/docs/images/demo-slideshow.mp4 index bacb43a..e7993e2 100644 Binary files a/docs/images/demo-slideshow.mp4 and b/docs/images/demo-slideshow.mp4 differ diff --git a/docs/images/demo-slideshow.webp b/docs/images/demo-slideshow.webp index 486a307..52da13f 100644 Binary files a/docs/images/demo-slideshow.webp and b/docs/images/demo-slideshow.webp differ diff --git a/docs/images/password-audit-slideshow.mp4 b/docs/images/password-audit-slideshow.mp4 index 9bc6020..a55a565 100644 Binary files a/docs/images/password-audit-slideshow.mp4 and b/docs/images/password-audit-slideshow.mp4 differ diff --git a/docs/images/password-audit-slideshow.webp b/docs/images/password-audit-slideshow.webp index 5eb6f69..7ad6922 100644 Binary files a/docs/images/password-audit-slideshow.webp and b/docs/images/password-audit-slideshow.webp differ diff --git a/modules/mssql_post_import.py b/modules/mssql_post_import.py index a04f1de..a69f8df 100644 --- a/modules/mssql_post_import.py +++ b/modules/mssql_post_import.py @@ -76,16 +76,21 @@ def canonicalize_mssql_linked_server_edges(connection) -> int: AND stub.objectid IS NOT NULL AND stub.objectid CONTAINS ':' WITH old, target, split(stub.objectid, ':')[0] AS sourceSid + CALL {{ + WITH sourceSid + MATCH (sidServer:MSSQL_Server) + WHERE sidServer.objectid IS NOT NULL + AND sidServer.objectid CONTAINS ':' + AND split(sidServer.objectid, ':')[0] = sourceSid + RETURN count(sidServer) AS sidCount + }} + WITH old, target, sourceSid, sidCount + WHERE sidCount = 1 MATCH (source:MSSQL_Server) WHERE source.objectid IS NOT NULL AND source.objectid CONTAINS ':' AND split(source.objectid, ':')[0] = sourceSid AND source <> target - AND COUNT {{ - MATCH (sidServer:MSSQL_Server) - WHERE sidServer.objectid CONTAINS ':' - AND split(sidServer.objectid, ':')[0] = sourceSid - }} = 1 MERGE (source)-[new:`{rtype}`]->(target) WITH old, new, properties(old) AS old_props, properties(new) AS existing_props SET new += old_props SET new += existing_props diff --git a/modules/neo4j_data.py b/modules/neo4j_data.py index 456b43e..aa45cca 100644 --- a/modules/neo4j_data.py +++ b/modules/neo4j_data.py @@ -1020,40 +1020,28 @@ def get_bad_successor_ou_privileges(self, force_refresh=False): if not force_refresh and hasattr(self, 'bad_successor_cache') and self.bad_successor_cache is not None: return self.bad_successor_cache - # no 2025 DC = no BadSuccessor; skip the OU scan - dc_check_query = """ - MATCH (dc:Computer)-[:MemberOf*1..]->(g:Group) - WHERE dc.operatingsystem =~ '(?i).*WINDOWS SERVER 2025.*' - AND g.name =~ '(?i).*DOMAIN CONTROLLERS.*' - AND dc.enabled = true - RETURN count(DISTINCT dc) as dc_count - """ - - dc_result = self.conn.query(dc_check_query, name="get_bad_successor_dc_check") - - if not dc_result or not dc_result[0] or dc_result[0].get('dc_count', 0) == 0: - self.bad_successor_cache = [] - return self.bad_successor_cache + dc_domain = self._domain_condition("dc") + target_domain = self._domain_condition("target") - badsuccessor_query = """ + badsuccessor_query = f""" MATCH (dc:Computer)-[:MemberOf*1..]->(g:Group) WHERE dc.operatingsystem =~ '(?i).*WINDOWS SERVER 2025.*' AND g.name =~ '(?i).*DOMAIN CONTROLLERS.*' - AND dc.enabled = true + AND dc.enabled = true{dc_domain} WITH count(DISTINCT dc) > 0 AS has2025DC WHERE has2025DC = true - // Query for OU privileges only - BadSuccessor focuses on OU control - MATCH p = (ou:OU)<-[r:WriteDacl|Owns|GenericAll|WriteOwner]-(n:Base) - WHERE NOT ((n:Tag_Tier_Zero) OR COALESCE(n.system_tags, '') CONTAINS 'admin_tier_0') - RETURN 'OU' AS target_type, - ou.name AS target_name, - n.name AS entity_name, - n.objectid AS entity_sid, - labels(n) AS entity_labels, - type(r) AS relationship_type, - null AS gpo_linked_domain, - ou.name AS ou_name + MATCH (target)<-[r:WriteDacl|Owns|GenericAll|WriteOwner]-(n) + WHERE (target:OU OR target:Container){target_domain} + AND (n:User OR n:Computer OR n:Group) + AND NOT ((n:Tag_Tier_Zero) OR COALESCE(n.system_tags, '') CONTAINS 'admin_tier_0') + AND COALESCE(n.enabled, true) = true + RETURN CASE WHEN target:OU THEN 'OU' ELSE 'Container' END AS target_type, + coalesce(target.name, target.objectid, 'Unknown') AS target_name, + coalesce(n.name, n.objectid, 'Unknown') AS entity_name, + target.objectid AS target_id, + n.objectid AS entity_id, + type(r) AS relationship_type """ result = self.conn.query(badsuccessor_query, name="get_bad_successor_ou_privileges") diff --git a/tests/checks/test_bad_successor.py b/tests/checks/test_bad_successor.py index 7db247a..1568aee 100644 --- a/tests/checks/test_bad_successor.py +++ b/tests/checks/test_bad_successor.py @@ -6,13 +6,42 @@ pytestmark = pytest.mark.neo4j -def test_fires_on_default_group_with_ou_privilege(clean_neo4j): +def test_flags_non_tier0_control_of_ous_and_containers(clean_neo4j): load_fixture(clean_neo4j, "bad_successor.cypher") findings = run_check(BadSuccessorCheck, clean_neo4j, domain_filter="TEST.LOCAL") assert BadSuccessorCheck.RISK_LEVEL == "High" - assert len(findings) == 1 + assert len(findings) == 2 - key = list(findings.keys())[0] - assert "DOMAIN USERS" in key and "GENERICALL" in key - assert isinstance(findings[key], dict) and "inline_description" in findings[key] + assert "DOMAIN USERS@TEST.LOCAL HAS GENERICALL ON OU: WORKSTATIONS@TEST.LOCAL" in findings + assert ("HELPDESK@TEST.LOCAL HAS WRITEDACL ON CONTAINER: " + "MANAGED SERVICE ACCOUNTS@TEST.LOCAL") in findings + + keys = "\n".join(findings) + assert "STALE" not in keys # disabled principal excluded + assert "DOMAIN ADMINS" not in keys # Tier-0 principal excluded + assert "OTHER.LOCAL" not in keys # different-domain targets excluded + assert "LinkedServer" not in keys # non-principal Base/OpenGraph nodes excluded + + sample = findings["DOMAIN USERS@TEST.LOCAL HAS GENERICALL ON OU: WORKSTATIONS@TEST.LOCAL"] + assert isinstance(sample, dict) and "inline_description" in sample + + +def test_falls_back_to_objectids_for_missing_names(clean_neo4j): + load_fixture(clean_neo4j, "bad_successor_fallback_names.cypher") + + findings = run_check(BadSuccessorCheck, clean_neo4j, domain_filter="TEST.LOCAL") + keys = "\n".join(findings) + + assert len(findings) == 3 + assert "None" not in keys + assert "S-1-5-21-TEST-2001 HAS GENERICALL ON OU: WORKSTATIONS@TEST.LOCAL" in findings + assert "S-1-5-21-TEST-2002 HAS GENERICALL ON OU: WORKSTATIONS@TEST.LOCAL" in findings + assert "NAMED@TEST.LOCAL HAS GENERICALL ON OU: TEST-OU-NONAME" in findings + + +def test_requires_2025_dc_in_filtered_domain(clean_neo4j): + load_fixture(clean_neo4j, "bad_successor_foreign_dc.cypher") + + findings = run_check(BadSuccessorCheck, clean_neo4j, domain_filter="TEST.LOCAL") + assert findings == {} diff --git a/tests/checks/test_sccm_takeover1.py b/tests/checks/test_sccm_takeover1.py index 9ac8d61..688c236 100644 --- a/tests/checks/test_sccm_takeover1.py +++ b/tests/checks/test_sccm_takeover1.py @@ -14,3 +14,10 @@ def test_fires_on_mssql_relay_to_sccm_db(clean_neo4j): assert len(findings) == 1 assert "S-1-5-21-TEST-1700" in findings assert isinstance(findings["S-1-5-21-TEST-1700"], str) + + +def test_excludes_foreign_and_disabled_coercion_sources(clean_neo4j): + load_fixture(clean_neo4j, "sccm_takeover1_source_filter.cypher") + + findings = run_check(SCCMTakeover1Check, clean_neo4j, domain_filter="TEST.LOCAL") + assert findings == {} diff --git a/tests/checks/test_sccm_takeover7.py b/tests/checks/test_sccm_takeover7.py index 59d17d6..a027dc4 100644 --- a/tests/checks/test_sccm_takeover7.py +++ b/tests/checks/test_sccm_takeover7.py @@ -14,3 +14,10 @@ def test_fires_on_ha_site_server_relay(clean_neo4j): assert len(findings) == 1 assert "S-1-5-21-TEST-1601" in findings assert isinstance(findings["S-1-5-21-TEST-1601"], str) + + +def test_no_finding_when_second_site_server_disabled(clean_neo4j): + load_fixture(clean_neo4j, "sccm_takeover7_disabled_host.cypher") + + findings = run_check(SCCMTakeover7Check, clean_neo4j, domain_filter="TEST.LOCAL") + assert not findings, "disabled site server must not count toward the 2+ HA threshold" diff --git a/tests/fixtures/bad_successor.cypher b/tests/fixtures/bad_successor.cypher index 7a7bf71..79e38e0 100644 --- a/tests/fixtures/bad_successor.cypher +++ b/tests/fixtures/bad_successor.cypher @@ -1,41 +1,120 @@ -// Positive: a Windows Server 2025 DC exists, and a well-known default group -// (Domain Users) has GenericAll on an OU — the BadSuccessor primitive. +// BadSuccessor: a Windows Server 2025 DC opens the gate, then the check flags +// every enabled, non-Tier-0 principal with full control over an OU or Container. +// Positives: Domain Users -> OU, Helpdesk -> Container. +// Negatives: a disabled user, a Tier-0 group, and a different-domain OU are excluded. -CREATE (dc:Computer { +CREATE (dc:Computer:Base { name: 'DC2025.TEST.LOCAL', objectid: 'S-1-5-21-TEST-4101', samaccountname: 'DC2025$', enabled: true, domain: 'TEST.LOCAL', - operatingsystem: 'Windows Server 2025' + operatingsystem: 'Windows Server 2025 Standard' }); -CREATE (dcGroup:Group { +CREATE (dcGroup:Group:Base { name: 'DOMAIN CONTROLLERS@TEST.LOCAL', objectid: 'S-1-5-21-TEST-516', domain: 'TEST.LOCAL' }); -CREATE (ou:OU { - name: 'WORKSTATIONS', - objectid: 'S-1-5-21-TEST-OU-1' +CREATE (otherDc:Computer:Base { + name: 'DC2025.OTHER.LOCAL', + objectid: 'S-1-5-21-OTHER-4101', + samaccountname: 'DC2025$', + enabled: true, + domain: 'OTHER.LOCAL', + operatingsystem: 'Windows Server 2025 Standard' +}); + +CREATE (otherDcGroup:Group:Base { + name: 'DOMAIN CONTROLLERS@OTHER.LOCAL', + objectid: 'S-1-5-21-OTHER-516', + domain: 'OTHER.LOCAL' +}); + +CREATE (ou:OU:Base { + name: 'WORKSTATIONS@TEST.LOCAL', + objectid: 'TEST-OU-WORKSTATIONS', + domain: 'TEST.LOCAL' +}); + +CREATE (msa:Container:Base { + name: 'MANAGED SERVICE ACCOUNTS@TEST.LOCAL', + objectid: 'TEST-CN-MSA', + domain: 'TEST.LOCAL' }); -// Domain Users needs the Base label for the OU privilege query. -// Realistic numeric SID required — bad_successor.py:57 checks the character -// before the RID suffix is a digit or '-', so S-1-5-21-TEST-513 fails. -CREATE (g:Group:Base { +CREATE (du:Group:Base { name: 'DOMAIN USERS@TEST.LOCAL', - objectid: 'S-1-5-21-1111111111-2222222222-3333333333-513', + objectid: 'S-1-5-21-TEST-513', + domain: 'TEST.LOCAL' +}); + +CREATE (helpdesk:Group:Base { + name: 'HELPDESK@TEST.LOCAL', + objectid: 'S-1-5-21-TEST-1601', domain: 'TEST.LOCAL' }); -// DC is member of Domain Controllers +CREATE (stale:User:Base { + name: 'STALE.OPERATOR@TEST.LOCAL', + objectid: 'S-1-5-21-TEST-1602', + domain: 'TEST.LOCAL', + enabled: false +}); + +CREATE (da:Group:Base:Tag_Tier_Zero { + name: 'DOMAIN ADMINS@TEST.LOCAL', + objectid: 'S-1-5-21-TEST-512', + domain: 'TEST.LOCAL' +}); + +CREATE (otherOu:OU:Base { + name: 'OTHER WORKSTATIONS@OTHER.LOCAL', + objectid: 'OTHER-OU-WORKSTATIONS', + domain: 'OTHER.LOCAL' +}); + +CREATE (otherHelpdesk:Group:Base { + name: 'OTHER HELPDESK@OTHER.LOCAL', + objectid: 'S-1-5-21-OTHER-1601', + domain: 'OTHER.LOCAL' +}); + +CREATE (stub:MSSQL_Base:OpenGraph_Stub:Base { + name: 'LinkedServer:SQL01', + objectid: 'MSSQL-STUB-1' +}); + MATCH (dc:Computer {objectid: 'S-1-5-21-TEST-4101'}), (dcGroup:Group {objectid: 'S-1-5-21-TEST-516'}) CREATE (dc)-[:MemberOf]->(dcGroup); -// Domain Users has GenericAll on the OU -MATCH (ou:OU {objectid: 'S-1-5-21-TEST-OU-1'}), - (g:Group {objectid: 'S-1-5-21-1111111111-2222222222-3333333333-513'}) -CREATE (ou)<-[:GenericAll]-(g); +MATCH (otherDc:Computer {objectid: 'S-1-5-21-OTHER-4101'}), + (otherDcGroup:Group {objectid: 'S-1-5-21-OTHER-516'}) +CREATE (otherDc)-[:MemberOf]->(otherDcGroup); + +MATCH (du:Group {objectid: 'S-1-5-21-TEST-513'}), + (ou:OU {objectid: 'TEST-OU-WORKSTATIONS'}) +CREATE (du)-[:GenericAll]->(ou); + +MATCH (helpdesk:Group {objectid: 'S-1-5-21-TEST-1601'}), + (msa:Container {objectid: 'TEST-CN-MSA'}) +CREATE (helpdesk)-[:WriteDacl]->(msa); + +MATCH (stale:User {objectid: 'S-1-5-21-TEST-1602'}), + (ou:OU {objectid: 'TEST-OU-WORKSTATIONS'}) +CREATE (stale)-[:GenericAll]->(ou); + +MATCH (da:Group {objectid: 'S-1-5-21-TEST-512'}), + (ou:OU {objectid: 'TEST-OU-WORKSTATIONS'}) +CREATE (da)-[:GenericAll]->(ou); + +MATCH (otherHelpdesk:Group {objectid: 'S-1-5-21-OTHER-1601'}), + (otherOu:OU {objectid: 'OTHER-OU-WORKSTATIONS'}) +CREATE (otherHelpdesk)-[:GenericAll]->(otherOu); + +MATCH (stub:MSSQL_Base {objectid: 'MSSQL-STUB-1'}), + (ou:OU {objectid: 'TEST-OU-WORKSTATIONS'}) +CREATE (stub)-[:GenericAll]->(ou); diff --git a/tests/fixtures/bad_successor_fallback_names.cypher b/tests/fixtures/bad_successor_fallback_names.cypher new file mode 100644 index 0000000..ec5088f --- /dev/null +++ b/tests/fixtures/bad_successor_fallback_names.cypher @@ -0,0 +1,63 @@ +// BadSuccessor regression: missing names should fall back to objectid and +// distinct nameless principals should not collapse into one finding. + +CREATE (dc:Computer:Base { + name: 'DC2025.TEST.LOCAL', + objectid: 'S-1-5-21-TEST-4101', + samaccountname: 'DC2025$', + enabled: true, + domain: 'TEST.LOCAL', + operatingsystem: 'Windows Server 2025 Standard' +}); + +CREATE (dcGroup:Group:Base { + name: 'DOMAIN CONTROLLERS@TEST.LOCAL', + objectid: 'S-1-5-21-TEST-516', + domain: 'TEST.LOCAL' +}); + +CREATE (ou:OU:Base { + name: 'WORKSTATIONS@TEST.LOCAL', + objectid: 'TEST-OU-WORKSTATIONS', + domain: 'TEST.LOCAL' +}); + +CREATE (namelessOu:OU:Base { + objectid: 'TEST-OU-NONAME', + domain: 'TEST.LOCAL' +}); + +CREATE (u1:User:Base { + objectid: 'S-1-5-21-TEST-2001', + domain: 'TEST.LOCAL', + enabled: true +}); + +CREATE (u2:User:Base { + objectid: 'S-1-5-21-TEST-2002', + domain: 'TEST.LOCAL', + enabled: true +}); + +CREATE (named:User:Base { + name: 'NAMED@TEST.LOCAL', + objectid: 'S-1-5-21-TEST-2003', + domain: 'TEST.LOCAL', + enabled: true +}); + +MATCH (dc:Computer {objectid: 'S-1-5-21-TEST-4101'}), + (dcGroup:Group {objectid: 'S-1-5-21-TEST-516'}) +CREATE (dc)-[:MemberOf]->(dcGroup); + +MATCH (u1:User {objectid: 'S-1-5-21-TEST-2001'}), + (ou:OU {objectid: 'TEST-OU-WORKSTATIONS'}) +CREATE (u1)-[:GenericAll]->(ou); + +MATCH (u2:User {objectid: 'S-1-5-21-TEST-2002'}), + (ou:OU {objectid: 'TEST-OU-WORKSTATIONS'}) +CREATE (u2)-[:GenericAll]->(ou); + +MATCH (named:User {objectid: 'S-1-5-21-TEST-2003'}), + (namelessOu:OU {objectid: 'TEST-OU-NONAME'}) +CREATE (named)-[:GenericAll]->(namelessOu); diff --git a/tests/fixtures/bad_successor_foreign_dc.cypher b/tests/fixtures/bad_successor_foreign_dc.cypher new file mode 100644 index 0000000..08046f5 --- /dev/null +++ b/tests/fixtures/bad_successor_foreign_dc.cypher @@ -0,0 +1,37 @@ +// BadSuccessor negative: a foreign Windows Server 2025 DC must not open the +// gate for vulnerable OU/container ACLs in TEST.LOCAL. + +CREATE (otherDc:Computer:Base { + name: 'DC2025.OTHER.LOCAL', + objectid: 'S-1-5-21-OTHER-4101', + samaccountname: 'DC2025$', + enabled: true, + domain: 'OTHER.LOCAL', + operatingsystem: 'Windows Server 2025 Standard' +}); + +CREATE (otherDcGroup:Group:Base { + name: 'DOMAIN CONTROLLERS@OTHER.LOCAL', + objectid: 'S-1-5-21-OTHER-516', + domain: 'OTHER.LOCAL' +}); + +CREATE (testOu:OU:Base { + name: 'TEST ONLY OU@TEST.LOCAL', + objectid: 'TEST-OU-FOREIGN-DC-GATE', + domain: 'TEST.LOCAL' +}); + +CREATE (testHelpdesk:Group:Base { + name: 'TEST HELPDESK@TEST.LOCAL', + objectid: 'S-1-5-21-TEST-1701', + domain: 'TEST.LOCAL' +}); + +MATCH (otherDc:Computer {objectid: 'S-1-5-21-OTHER-4101'}), + (otherDcGroup:Group {objectid: 'S-1-5-21-OTHER-516'}) +CREATE (otherDc)-[:MemberOf]->(otherDcGroup); + +MATCH (testHelpdesk:Group {objectid: 'S-1-5-21-TEST-1701'}), + (testOu:OU {objectid: 'TEST-OU-FOREIGN-DC-GATE'}) +CREATE (testHelpdesk)-[:GenericAll]->(testOu); diff --git a/tests/fixtures/sccm_takeover1_source_filter.cypher b/tests/fixtures/sccm_takeover1_source_filter.cypher new file mode 100644 index 0000000..1b30580 --- /dev/null +++ b/tests/fixtures/sccm_takeover1_source_filter.cypher @@ -0,0 +1,64 @@ +// TAKEOVER-1 negative: out-of-domain and disabled AD coercion sources must not +// create a finding for an in-scope SCCM site database path. + +CREATE (site:SCCM_Site {siteCode: 'P01', sourceForest: 'TEST.LOCAL'}); +CREATE (server:MSSQL_Server {name: 'sql01.test.local:1433', extendedProtection: false}); + +CREATE (login:MSSQL_Login {name: 'TEST\\sccmadmin', SQLServer: 'sql01.test.local:1433', type: 'WINDOWS_LOGIN'}); +CREATE (role:MSSQL_ServerRole {name: 'sysadmin', SQLServer: 'sql01.test.local:1433'}); +CREATE (db:MSSQL_Database {name: 'CM_P01', SQLServer: 'sql01.test.local:1433'}); + +CREATE (foreignGroup:Group { + name: 'FOREIGN HELPERS@OTHER.LOCAL', + objectid: 'S-1-5-21-OTHER-1701', + domain: 'OTHER.LOCAL' +}); + +CREATE (disabledSource:Computer { + name: 'DISABLED-SOURCE.TEST.LOCAL', + objectid: 'S-1-5-21-TEST-1702', + samaccountname: 'DISABLED-SOURCE$', + enabled: false, + domain: 'TEST.LOCAL' +}); + +CREATE (target:Computer { + name: 'SQLHOST.TEST.LOCAL', + DNSHostName: 'sqlhost.test.local', + samaccountname: 'SQLHOST$', + objectid: 'S-1-5-21-TEST-1700', + enabled: true, + domain: 'TEST.LOCAL' +}); + +MATCH (foreignGroup:Group {objectid: 'S-1-5-21-OTHER-1701'}), + (login:MSSQL_Login {name: 'TEST\\sccmadmin'}) +CREATE (foreignGroup)-[:CoerceAndRelayToMSSQL { + coercionVictimAndRelayTargetPairs: ['Coerce foreign.other.local, relay to sql01.test.local:1433'] +}]->(login); + +MATCH (disabledSource:Computer {objectid: 'S-1-5-21-TEST-1702'}), + (login:MSSQL_Login {name: 'TEST\\sccmadmin'}) +CREATE (disabledSource)-[:CoerceAndRelayToMSSQL { + coercionVictimAndRelayTargetPairs: ['Coerce disabled-source.test.local, relay to sql01.test.local:1433'] +}]->(login); + +MATCH (login:MSSQL_Login {name: 'TEST\\sccmadmin'}), + (role:MSSQL_ServerRole {name: 'sysadmin'}) +CREATE (login)-[:MSSQL_MemberOf]->(role); + +MATCH (role:MSSQL_ServerRole {name: 'sysadmin'}), + (server:MSSQL_Server {name: 'sql01.test.local:1433'}) +CREATE (role)-[:MSSQL_ControlServer]->(server); + +MATCH (login:MSSQL_Login {name: 'TEST\\sccmadmin'}), + (db:MSSQL_Database {name: 'CM_P01'}) +CREATE (login)-[:MSSQL_ControlDB]->(db); + +MATCH (db:MSSQL_Database {name: 'CM_P01'}), + (site:SCCM_Site {siteCode: 'P01'}) +CREATE (db)-[:SCCM_AssignAllPermissions]->(site); + +MATCH (target:Computer {objectid: 'S-1-5-21-TEST-1700'}), + (server:MSSQL_Server {name: 'sql01.test.local:1433'}) +CREATE (target)-[:MSSQL_HostFor]->(server); diff --git a/tests/fixtures/sccm_takeover7_disabled_host.cypher b/tests/fixtures/sccm_takeover7_disabled_host.cypher new file mode 100644 index 0000000..edacacc --- /dev/null +++ b/tests/fixtures/sccm_takeover7_disabled_host.cypher @@ -0,0 +1,28 @@ +// TAKEOVER-7 negative: one live site server plus one decommissioned (disabled) +// site server for the same site. The disabled host carries a stale +// SCCMSiteSystemRoles value but no longer exists, so it must not count toward +// the 2+ HA-server threshold — the check must return no finding. + +CREATE (site:SCCM_Site {siteCode: 'P01', sourceForest: 'TEST.LOCAL'}); + +CREATE (live:Computer { + name: 'SITESVR1.TEST.LOCAL', + DNSHostName: 'sitesvr1.test.local', + samaccountname: 'SITESVR1$', + objectid: 'S-1-5-21-TEST-1600', + enabled: true, + domain: 'TEST.LOCAL', + SMBSigningRequired: false, + SCCMSiteSystemRoles: ['SMS Site Server@P01'] +}); + +CREATE (stale:Computer { + name: 'OLDSITESVR.TEST.LOCAL', + DNSHostName: 'oldsitesvr.test.local', + samaccountname: 'OLDSITESVR$', + objectid: 'S-1-5-21-TEST-1699', + enabled: false, + domain: 'TEST.LOCAL', + SMBSigningRequired: false, + SCCMSiteSystemRoles: ['SMS Site Server@P01'] +}); diff --git a/tests/test_check_inventory.py b/tests/test_check_inventory.py index d47af80..aab1a05 100644 --- a/tests/test_check_inventory.py +++ b/tests/test_check_inventory.py @@ -14,6 +14,9 @@ EXTRA_TEST_FILES = {"test_manager_smoke.py"} EXTRA_FIXTURES = { + "bad_successor_foreign_dc", + "bad_successor_fallback_names", + "sccm_takeover7_disabled_host", "esc1_vulnerable_template_manager_approval", "esc3_enrollment_agent_manager_approval", "esc6_user_specifies_san_manager_approval", @@ -31,6 +34,7 @@ "mssql_priv_esc_linked_server_empty_stub", "mssql_priv_esc_linked_server_shared_host_sid", "mssql_priv_esc_owns_server_role", + "sccm_takeover1_source_filter", "sccm_priv_esc_linked_server_name_overmatch", "sccm_priv_esc_linked_server_empty_stub", "sccm_priv_esc_linked_server_shared_host_sid",