fix(implant): services list panics on nil error on Windows (#1989)#2249
Merged
moloch-- merged 1 commit intoBishopFox:masterfrom Apr 22, 2026
Merged
Conversation
9d617c7 to
2cb9891
Compare
servicesListHandler called err.Error() unconditionally after service.ListServices(). On a healthy Windows host ListServices returns nil error, causing a nil pointer dereference that crashed the implant. serviceDetailHandler in the same file already guards correctly with if err != nil. Apply the same pattern to servicesListHandler by extracting response construction into a buildServicesResp helper with a proper nil check. Verified on Windows 11 VM: unfixed helper panics with "runtime error: invalid memory address or nil pointer dereference" on the nil-error path; fixed helper passes. Fixes BishopFox#1989
2cb9891 to
f83dc42
Compare
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
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.
Bug
Running
servicesagainst a Windows beacon can kill the beacon with a nil pointer deref. The implant panics and the session dies.Cause
servicesListHandlerinimplant/sliver/handlers/handlers_windows.gocallserr.Error()with no nil check:ListServicesreturns(list, nil)when every service reads without error. Calling.Error()on nil →0xc0000005, process exits.Fix
Add the nil guard. One helper, one check:
Why it doesn't repro easily
A beacon running as a normal user hits access-denied on a bunch of services during
openService/Config()/Query(). Those failures get collected intoserviceErrorsandListServicesreturns a non-nil error — so the buggyerr.Error()call is safe. That's why most operators never see this crash.The bug only fires when every service reads cleanly (no errors at all). On modern Win10/11 that's hard to hit even as SYSTEM because services like
WaaSMedicSvcare PPL-protected and will fail for anyone. But it's not impossible — older Windows versions, stripped-down VMs, or just the right host config hits it and the beacon dies.Reproducing it deliberately
To force the code path, add one line to
ListServices(implant/sliver/service/service_windows.go, near line 195):Then build sliver-server, generate a Windows beacon, run it on any Win VM, and call
services— the unpatched beacon panics, the patched beacon returns results normally.Fixes #1989