-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix Windows network adapter retrieval for DNS C2, add additional logging #1963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
… server info, added additional logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR increases the Windows adapter buffer size to prevent overflow, enriches DNS resolver and adapter enumeration logs for easier debugging, and restores the dns
CLI command.
- Bump DNS adapter buffer from 20 KB to 1 MB
- Add detailed
log.Printf
calls inresolver-generic.go
andconf_windows.go
- Reintroduce
dnsCmd
in the client command set
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
implant/sliver/transports/dnsclient/resolver-generic.go | Expanded and clarified logging for A, AAAA, TXT lookups and localQuery |
implant/sliver/transports/dnsclient/conf_windows.go | Increased buffer size, imported fmt , and added verbose adapter/DNS server logging |
client/command/jobs/commands.go | Added dnsCmd to the returned list of client commands |
Comments suppressed due to low confidence (2)
implant/sliver/transports/dnsclient/resolver-generic.go:85
- [nitpick] Referencing the internal function name
a()
in logs may confuse readers. Consider describing the action (e.g., 'using localQuery TypeA') instead of the function name.
log.Printf("[dns] error retrieving A records for '%s' using a(): %v (retry wait: %s)", domain, err, r.retryWait)
client/command/jobs/commands.go:166
- Ensure that
dnsCmd
is defined and initialized in this package; otherwise, adding it here will cause a compile-time error.
return []*cobra.Command{jobsCmd, mtlsCmd, wgCmd, dnsCmd, httpCmd, httpsCmd, stageCmd}
adapterAddressString := "" | ||
adapterIPAddress := addr.FirstUnicastAddress.Address.IP() | ||
if adapterIPAddress != nil { | ||
adapterAddressString = fmt.Sprintf(" with address %s", adapterIPAddress.To16()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using adapterIPAddress.To16()
prints the raw 16-byte slice, which may not render cleanly. Consider using adapterIPAddress.String()
to produce a human-readable IP address.
adapterAddressString = fmt.Sprintf(" with address %s", adapterIPAddress.To16()) | |
adapterAddressString = fmt.Sprintf(" with address %s", adapterIPAddress.String()) |
Copilot uses AI. Check for mistakes.
b := make([]byte, l) | ||
|
||
// Windows is an utter fucking trash fire of an operating system. | ||
if err := windows.GetAdaptersAddresses(windows.AF_UNSPEC, windows.GAA_FLAG_INCLUDE_PREFIX, 0, (*windows.IpAdapterAddresses)(unsafe.Pointer(&b[0])), &l); err != nil { | ||
log.Printf("[dns] error getting Windows network adapter addresses: %v. If the error message is 'The file name is too long', this indicates a buffer overflow due to the data exceeding the size of the buffer allocated in the dnsClientConfig function of sliver/implant/sliver/transports/dnsclient/conf_windows.go.", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This log entry is very verbose for production. Consider reducing it to a concise error message or moving the detailed explanation under a debug flag.
log.Printf("[dns] error getting Windows network adapter addresses: %v. If the error message is 'The file name is too long', this indicates a buffer overflow due to the data exceeding the size of the buffer allocated in the dnsClientConfig function of sliver/implant/sliver/transports/dnsclient/conf_windows.go.", err) | |
log.Printf("[dns] error getting Windows network adapter addresses: %v", err) | |
// {{if .Config.Debug}} | |
log.Printf("[dns] Debug info: If the error message is 'The file name is too long', this indicates a buffer overflow due to the data exceeding the size of the buffer allocated in the dnsClientConfig function.") | |
// {{end}} |
Copilot uses AI. Check for mistakes.
Windows Platform not working [] Generating new windows/amd64 implant binary [server] sliver > |
This PR increases the size of the buffer that's used to retrieve Windows network adapter information as part of establishing a DNS C2 connection. In prior versions, the buffer was 20K in size, which was insufficient. When
windows.IpAdapterAddresses
encounters this situation, it returns the POSIX error code for "The file name is too long", because that's the closest POSIX has to a "buffer overflow" code.[1]This was the cause of #1411
I imagine it was hard to reproduce because it only occurred on systems with a sufficient number of network adapters.
I increased the buffer size to 1MB, which I think would be enough on just about any Windows system.
I also added some additional logging and more information to a few existing messages to make troubleshooting DNS C2 easier.
The PR also re-adds the
dns
command, since that was necessary to do the dev work.[1] See this discussion related to MSYS from 2011 for the same symptom: https://groups.google.com/g/msysgit/c/e9dkPpl2C8E/m/P0cpB8KS8oAJ