Skip to content

Commit a196624

Browse files
author
r0BIT
committed
Fix B7-B9: LDAP domain validation, filtered task count, early SID skip
B7 - LDAP Empty Domain Bug (FIXED): - Added domain validation to resolve_name_to_sid_via_ldap(), resolve_sid_via_ldap(), batch_get_user_attributes(), and fetch_tier0_members() - Functions return None/empty early if domain is empty or doesn't contain dots - Prevents invalidDNSyntax errors when base DN becomes 'DC=' - Added 7 new tests for empty domain handling B8 - TaskCount Shows Filtered vs Total (FIXED): - Added filtered_count counter that increments after should_include check - Updated TaskCount output: '15 domain tasks (120 total), 0 Privileged' - Clearer UX - users now see how many tasks passed the filter vs raw total B9 - Early Skip for Well-Known SIDs (FIXED): - Added early skip check before SID resolution using looks_like_domain_user() - Skips resolution for S-1-5-18 (SYSTEM), S-1-5-19, S-1-5-20 when include_local=False - Avoids unnecessary cache lookups for tasks that will be filtered anyway
1 parent 1460ec3 commit a196624

File tree

3 files changed

+141
-25
lines changed

3 files changed

+141
-25
lines changed

taskhound/engine/online.py

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,7 @@ def process_target(
552552
traceback.print_exc()
553553

554554
total = len(items)
555+
filtered_count = 0 # Count of tasks that pass should_include filter
555556
priv_count = 0
556557
priv_lines: List[str] = []
557558
task_lines: List[str] = []
@@ -702,29 +703,38 @@ def process_target(
702703
# Resolve SID early if runas is a SID - store result for credential matching and output
703704
# This ensures we only resolve once per task, and the result is available for all uses
704705
# Skipped in OPSEC mode to avoid SMB/LSARPC and LDAP queries
706+
# Also skip well-known local SIDs (S-1-5-18 SYSTEM, S-1-5-19, S-1-5-20) - they'll be filtered anyway
705707
if is_sid(runas) and not opsec:
706-
# Derive local domain SID prefix from computer SID for foreign domain detection
707-
from ..utils.sid_resolver import get_domain_sid_prefix
708-
local_domain_prefix = get_domain_sid_prefix(server_sid) if server_sid else None
709-
710-
_, row.resolved_runas = format_runas_with_sid_resolution(
711-
runas,
712-
hv_loader=hv,
713-
bh_connector=bh_connector,
714-
smb_connection=smb,
715-
no_ldap=no_ldap,
716-
domain=domain,
717-
dc_ip=dc_ip,
718-
username=username,
719-
password=password,
720-
hashes=hashes,
721-
kerberos=kerberos,
722-
ldap_domain=ldap_domain,
723-
ldap_user=ldap_user,
724-
ldap_password=ldap_password,
725-
ldap_hashes=ldap_hashes,
726-
local_domain_sid_prefix=local_domain_prefix,
727-
)
708+
# Skip SID resolution for well-known local SIDs that will be filtered out
709+
# This avoids unnecessary cache lookups for SYSTEM (S-1-5-18), etc.
710+
from ..utils.sid_resolver import looks_like_domain_user
711+
if not looks_like_domain_user(runas) and not include_local:
712+
# This SID is a local/system account and we're not including locals
713+
# Skip resolution - the task will be filtered out in classify_task()
714+
pass
715+
else:
716+
# Derive local domain SID prefix from computer SID for foreign domain detection
717+
from ..utils.sid_resolver import get_domain_sid_prefix
718+
local_domain_prefix = get_domain_sid_prefix(server_sid) if server_sid else None
719+
720+
_, row.resolved_runas = format_runas_with_sid_resolution(
721+
runas,
722+
hv_loader=hv,
723+
bh_connector=bh_connector,
724+
smb_connection=smb,
725+
no_ldap=no_ldap,
726+
domain=domain,
727+
dc_ip=dc_ip,
728+
username=username,
729+
password=password,
730+
hashes=hashes,
731+
kerberos=kerberos,
732+
ldap_domain=ldap_domain,
733+
ldap_user=ldap_user,
734+
ldap_password=ldap_password,
735+
ldap_hashes=ldap_hashes,
736+
local_domain_sid_prefix=local_domain_prefix,
737+
)
728738

729739
# Enrich row with decrypted password if available from DPAPI loot
730740
if decrypted_creds:
@@ -762,6 +772,8 @@ def process_target(
762772
if not result.should_include:
763773
continue
764774

775+
filtered_count += 1 # Task passed the filter
776+
765777
# Format output block based on classification
766778
if result.task_type in ("TIER-0", "PRIV"):
767779
priv_lines.extend(
@@ -841,8 +853,12 @@ def process_target(
841853

842854
priv_display = priv_count if (hv and hv.loaded) else 'N/A'
843855
status(f"[Collecting] {target} [+]")
844-
status(f"[TaskCount] {total} Tasks, {priv_display} Privileged")
845-
good(f"{target}: Found {total} tasks, privileged {priv_display}{backup_msg}{laps_msg}")
856+
# Show filtered count (domain tasks) vs total (all tasks including SYSTEM)
857+
if filtered_count < total:
858+
status(f"[TaskCount] {filtered_count} domain tasks ({total} total), {priv_display} Privileged")
859+
else:
860+
status(f"[TaskCount] {total} Tasks, {priv_display} Privileged")
861+
good(f"{target}: Found {filtered_count} tasks (of {total} total), privileged {priv_display}{backup_msg}{laps_msg}")
846862

847863
# Combine credential loot output with task listing output
848864
# Put credentials first since they're the most valuable

taskhound/utils/sid_resolver.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,11 @@ def resolve_sid_via_ldap(
333333
debug("No valid credentials provided for LDAP SID resolution")
334334
return None
335335

336+
# Validate domain - must be non-empty and contain at least one dot for LDAP DN construction
337+
if not domain or "." not in domain:
338+
debug(f"Invalid domain '{domain}' for LDAP SID resolution - must be FQDN")
339+
return None
340+
336341
# If no DC IP provided, try to resolve it
337342
if not dc_ip:
338343
try:
@@ -465,6 +470,11 @@ def resolve_name_to_sid_via_ldap(
465470
else:
466471
cache_key = None # Only cache computers for now
467472

473+
# Validate domain - must be non-empty and contain at least one dot for LDAP DN construction
474+
if not domain or "." not in domain:
475+
debug(f"Invalid domain '{domain}' for LDAP resolution - must be FQDN (e.g., 'corp.local')")
476+
return None
477+
468478
try:
469479
# Extract just the name part if it's in USER@DOMAIN format
470480
search_name = name
@@ -885,6 +895,11 @@ def batch_get_user_attributes(
885895
if not users_to_query:
886896
return {}
887897

898+
# Validate domain - must be non-empty and contain at least one dot for LDAP DN construction
899+
if not domain or "." not in domain:
900+
debug(f"Invalid domain '{domain}' for batch user attribute lookup - must be FQDN")
901+
return {}
902+
888903
# Check cache first
889904
cache = get_cache()
890905
results = {}
@@ -1211,7 +1226,9 @@ def fetch_tier0_members(
12111226
"""
12121227
tier0_cache: Tier0Cache = {}
12131228

1214-
if not domain:
1229+
# Validate domain - must be non-empty and contain at least one dot for LDAP DN construction
1230+
if not domain or "." not in domain:
1231+
debug(f"Invalid domain '{domain}' for Tier-0 pre-flight - must be FQDN")
12151232
return tier0_cache
12161233

12171234
# Check persistent cache first

tests/test_sid_resolver.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,3 +344,86 @@ def test_returns_false_when_no_local_prefix(self):
344344
foreign_sid = "S-1-5-21-999888777-666555444-333222111-1001"
345345

346346
assert is_foreign_domain_sid(foreign_sid, None) is False
347+
348+
349+
class TestLDAPDomainValidation:
350+
"""Tests for LDAP domain validation (B7 bug fix)"""
351+
352+
def test_resolve_name_to_sid_via_ldap_rejects_empty_domain(self):
353+
"""Should return None for empty domain (prevents invalidDNSyntax error)"""
354+
from taskhound.utils.sid_resolver import resolve_name_to_sid_via_ldap
355+
356+
# Empty domain should return None immediately without attempting LDAP
357+
result = resolve_name_to_sid_via_ldap(
358+
name="testcomputer",
359+
domain="",
360+
is_computer=True,
361+
)
362+
363+
assert result is None
364+
365+
def test_resolve_name_to_sid_via_ldap_rejects_domain_without_dots(self):
366+
"""Should return None for domain without dots (not FQDN)"""
367+
from taskhound.utils.sid_resolver import resolve_name_to_sid_via_ldap
368+
369+
# Single-label domain (no dots) should return None
370+
result = resolve_name_to_sid_via_ldap(
371+
name="testcomputer",
372+
domain="TESTDOMAIN",
373+
is_computer=True,
374+
)
375+
376+
assert result is None
377+
378+
def test_resolve_sid_via_ldap_rejects_empty_domain(self):
379+
"""Should return None for empty domain in SID resolution"""
380+
from taskhound.utils.sid_resolver import resolve_sid_via_ldap
381+
382+
result = resolve_sid_via_ldap(
383+
sid="S-1-5-21-123456789-987654321-111111111-1001",
384+
domain="",
385+
username="testuser",
386+
password="testpass",
387+
)
388+
389+
assert result is None
390+
391+
def test_resolve_sid_via_ldap_rejects_domain_without_dots(self):
392+
"""Should return None for domain without dots in SID resolution"""
393+
from taskhound.utils.sid_resolver import resolve_sid_via_ldap
394+
395+
result = resolve_sid_via_ldap(
396+
sid="S-1-5-21-123456789-987654321-111111111-1001",
397+
domain="NODOTS",
398+
username="testuser",
399+
password="testpass",
400+
)
401+
402+
assert result is None
403+
404+
def test_batch_get_user_attributes_rejects_empty_domain(self):
405+
"""Should return empty dict for empty domain in batch query"""
406+
from taskhound.utils.sid_resolver import batch_get_user_attributes
407+
408+
result = batch_get_user_attributes(
409+
usernames=["testuser"],
410+
domain="",
411+
)
412+
413+
assert result == {}
414+
415+
def test_fetch_tier0_members_rejects_empty_domain(self):
416+
"""Should return empty dict for empty domain in Tier-0 preflight"""
417+
from taskhound.utils.sid_resolver import fetch_tier0_members
418+
419+
result = fetch_tier0_members(domain="")
420+
421+
assert result == {}
422+
423+
def test_fetch_tier0_members_rejects_domain_without_dots(self):
424+
"""Should return empty dict for domain without dots"""
425+
from taskhound.utils.sid_resolver import fetch_tier0_members
426+
427+
result = fetch_tier0_members(domain="SINGLELABEL")
428+
429+
assert result == {}

0 commit comments

Comments
 (0)