Fix domain resolution for non-domain-joined runs#216
Open
ciyi wants to merge 1 commit intoSpecterOps:2.Xfrom
Open
Fix domain resolution for non-domain-joined runs#216ciyi wants to merge 1 commit intoSpecterOps:2.Xfrom
ciyi wants to merge 1 commit intoSpecterOps:2.Xfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
WalkthroughEnhanced domain resolution in Changes
Sequence Diagram(s)sequenceDiagram
participant Client as User/Client
participant Init as Initialize()
participant LDAP as LDAPUtils
participant Infer as TryInferDomainName()
participant Config as LdapConfig
Client->>Init: Start initialization with context
Init->>LDAP: GetDomain(out domain)
alt Domain resolved
LDAP-->>Init: Success
Init->>Client: Continue
else Domain blank or GetDomain fails
LDAP-->>Init: Failure/Blank
Init->>Infer: Attempt domain inference
Infer->>Config: Parse Username for `@domain` or DOMAIN\user
alt Username contains domain info
Config-->>Infer: Domain extracted
Infer-->>Init: Success
Init->>Client: Log info, continue
else Try Server hostname parsing
Infer->>Config: Parse Server for domain portion
alt Server domain extraction succeeds
Config-->>Infer: Domain extracted
Infer-->>Init: Success
Init->>Client: Log info, continue
else All inference fails
Infer-->>Init: Failure
Init->>Client: Log critical error, set IsFaulted=true
end
end
end
sequenceDiagram
participant Enum as GetDomainsForEnumeration()
participant LDAP as LDAPUtils
participant Async as Async SID Resolver
Enum->>LDAP: GetDomain(context.DomainName, out domain)
alt Domain object retrieved
LDAP-->>Enum: Success with DomainObject
Enum->>Enum: Extract SID from domain object
Enum->>Enum: Add to Domains collection
else GetDomain fails
LDAP-->>Enum: Failure/Null
Enum->>Async: GetDomainSidFromDomainName(domain) async
alt SID resolution succeeds
Async-->>Enum: Return SID
Enum->>Enum: Add domain with resolved SID
else SID resolution fails
Async-->>Enum: Failure
Enum->>Enum: Log warning
Enum->>Enum: Add domain with DomainSid = "Unknown"
end
end
Enum->>Enum: Continue enumeration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Apr 16, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+275
to
+281
| if (!string.IsNullOrWhiteSpace(options.Server)) { | ||
| var server = options.Server.Trim(); | ||
| var firstDot = server.IndexOf('.'); | ||
| if (firstDot > 0 && firstDot < server.Length - 1) { | ||
| domainName = server.Substring(firstDot + 1); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Parse options.Server before domain extraction to avoid invalid inferred domains.
Line 277 currently parses the raw server string. If Server contains a port or IP, inference can produce invalid domain values and break downstream SID lookup.
Proposed fix
if (!string.IsNullOrWhiteSpace(options.Server)) {
- var server = options.Server.Trim();
- var firstDot = server.IndexOf('.');
- if (firstDot > 0 && firstDot < server.Length - 1) {
- domainName = server.Substring(firstDot + 1);
+ var server = options.Server.Trim();
+ var host = server;
+
+ // Handle absolute forms like ldap://dc.test.local:389
+ if (server.Contains("://") && Uri.TryCreate(server, UriKind.Absolute, out var uri)) {
+ host = uri.Host;
+ } else {
+ // Handle host:port without scheme
+ var colon = server.LastIndexOf(':');
+ if (colon > 0 && server.IndexOf(':') == colon) {
+ host = server.Substring(0, colon);
+ }
+ }
+
+ // Skip IP literals; they cannot be used to infer a DNS domain safely
+ if (System.Net.IPAddress.TryParse(host, out _)) {
+ return false;
+ }
+
+ var firstDot = host.IndexOf('.');
+ if (firstDot > 0 && firstDot < host.Length - 1) {
+ domainName = host.Substring(firstDot + 1);
return true;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!string.IsNullOrWhiteSpace(options.Server)) { | |
| var server = options.Server.Trim(); | |
| var firstDot = server.IndexOf('.'); | |
| if (firstDot > 0 && firstDot < server.Length - 1) { | |
| domainName = server.Substring(firstDot + 1); | |
| return true; | |
| } | |
| if (!string.IsNullOrWhiteSpace(options.Server)) { | |
| var server = options.Server.Trim(); | |
| var host = server; | |
| // Handle absolute forms like ldap://dc.test.local:389 | |
| if (server.Contains("://") && Uri.TryCreate(server, UriKind.Absolute, out var uri)) { | |
| host = uri.Host; | |
| } else { | |
| // Handle host:port without scheme | |
| var colon = server.LastIndexOf(':'); | |
| if (colon > 0 && server.IndexOf(':') == colon) { | |
| host = server.Substring(0, colon); | |
| } | |
| } | |
| // Skip IP literals; they cannot be used to infer a DNS domain safely | |
| if (System.Net.IPAddress.TryParse(host, out _)) { | |
| return false; | |
| } | |
| var firstDot = host.IndexOf('.'); | |
| if (firstDot > 0 && firstDot < host.Length - 1) { | |
| domainName = host.Substring(firstDot + 1); | |
| return true; | |
| } | |
| } |
|
@ciyi we'll need you to agree to the CLA to consider this PR for inclusion in the product, thank you! |
Author
|
I have read the CLA Document and I hereby sign the CLA |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When newer versions of SharpHound are executed from a non-domain-joined machine, it often fails with: ERROR|Unable to resolve a domain to use, manually specify one or check spelling.
Fixed GetDomain to:
GetDomainsForEnumerationUnknownSIDReproduction
runas /netonlycontext works with:.\SharpHound.exe -c All --domaincontroller DC.test.local -d test.localrunas, using explicit LDAP credentials previously failed during domain resolution:.\SharpHound.exe -c All --domaincontroller DC.test.local --ldapusername user123 --ldappassword "Password1!" -d test.local.\SharpHound.exe -c All --domaincontroller DC.test.local --ldapusername user123@test.local --ldappassword "Password1!" -d test.localSummary by CodeRabbit
Bug Fixes