Conversation
pr1mu2
requested review from
Marshall-Hallenbeck,
NeffIsBack,
mpgn and
zblurx
as code owners
|
I love it. Thanks for adding SCShell concept to an already great tool. |
|
@Mr-Un1k0d3r what an amazing technique omgggggggggggg |
|
Thanks for the PR, definitely looks cool! |
NeffIsBack
left a comment
NeffIsBack
left a comment
There was a problem hiding this comment.
Definitely looks cool!
A few thoughts I had while reading the code: As far as i can tell we alter the path of the service binary to our executable (cmd/powershell), what happens if it fails to revert that? Do we just broke that computer? Could potentially be dangerous.
Also, this looks very similar to #1174, with the difference being that we upload&install our own service binary (psexec), is that correct @Dfte? What would be the advantage of using psexec over simply this technique? Maybe #1174 is even a special case of this general technique here and we could make a module that uses this service class here to abuse it with psexec? Maybe I am missing something tho.
| def __init__( | ||
| self, | ||
| target, | ||
| username="", | ||
| password="", | ||
| domain="", | ||
| doKerberos=False, | ||
| aesKey=None, | ||
| remoteHost=None, | ||
| kdcHost=None, | ||
| hashes=None, | ||
| logger=None, | ||
| service_name="RemoteRegistry", | ||
| no_cmd=False, | ||
| wait_time=5, | ||
| ): |
There was a problem hiding this comment.
Please put this into one line, similar to the other exec methods
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, |
There was a problem hiding this comment.
As far as I can tell these NULL values are default values so we can just omit them.
| monkeypatch.setattr( | ||
| sys, | ||
| "argv", | ||
| [ | ||
| "netexec", | ||
| "smb", | ||
| "127.0.0.1", | ||
| "-u", | ||
| "user", | ||
| "-p", | ||
| "pass", | ||
| "-x", | ||
| "whoami", | ||
| "--exec-method", | ||
| "scshell", | ||
| ], | ||
| ) |
| cmd_exec_group.add_argument("--get-output-tries", help="Number of times atexec/smbexec/mmcexec tries to get results", type=int, default=10) | ||
| cmd_exec_group.add_argument("--codec", default="utf-8", help="Set encoding used (codec) from the target's output. If errors are detected, run chcp.com at the target & map the result with https://docs.python.org/3/library/codecs.html#standard-encodings and then execute again with --codec and the corresponding codec") | ||
| cmd_exec_group.add_argument("--no-output", action="store_true", help="do not retrieve command output") | ||
| cmd_exec_group.add_argument("--sc-service-name", default="RemoteRegistry", help="service name to reconfigure temporarily when using --exec-method scshell") |
There was a problem hiding this comment.
It is always difficult to add arguments that only work with one exec method. Usually, these exec methods should be replaceable and therefore not have different options that could break/result in different behaviour.
What would be use cases where you would alter the service name? For stealth reasons? What if we implement something like an array of possible services and then we just randomly chose one?
| cmd_exec_group.add_argument("--codec", default="utf-8", help="Set encoding used (codec) from the target's output. If errors are detected, run chcp.com at the target & map the result with https://docs.python.org/3/library/codecs.html#standard-encodings and then execute again with --codec and the corresponding codec") | ||
| cmd_exec_group.add_argument("--no-output", action="store_true", help="do not retrieve command output") | ||
| cmd_exec_group.add_argument("--sc-service-name", default="RemoteRegistry", help="service name to reconfigure temporarily when using --exec-method scshell") | ||
| cmd_exec_group.add_argument("--sc-no-cmd", action="store_true", help="when using --exec-method scshell, do not prepend cmd.exe /c to the command") |
There was a problem hiding this comment.
Same as above, but what would we use that for? E.g. executing powershell.exe instead of cmd? Maybe we should just change the way the commands are generated then, so we have this ability (to just call powershell.exe without cmd.exe) in every exec-method.
| scmr.hRChangeServiceConfigW( | ||
| self.__scmr, | ||
| self.__serviceHandle, | ||
| scmr.SERVICE_NO_CHANGE, | ||
| scmr.SERVICE_DEMAND_START, | ||
| scmr.SERVICE_ERROR_IGNORE, | ||
| final_command, |
| scmr.hRChangeServiceConfigW( | ||
| self.__scmr, | ||
| self.__serviceHandle, | ||
| scmr.SERVICE_NO_CHANGE, | ||
| self.__startType, | ||
| self.__errorControl, | ||
| self.__binaryPath, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| ) |
| try: | ||
| scmr.hRCloseServiceHandle(self.__scmr, self.__serviceHandle) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Please no empty exception handling. Either contextlib.suppress or we should alert the user/log a debug message if there is something useful to log.
| try: | ||
| scmr.hRCloseServiceHandle(self.__scmr, self.__scHandle) | ||
| except Exception: | ||
| pass | ||
| finally: | ||
| self.__scHandle = None | ||
|
|
||
| if self.__scmr: | ||
| try: | ||
| self.__scmr.disconnect() | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
See above, probably all of that should just be:
with contextlib.suppress:
scmr.closeHandle()
scmr.close()
scmr.disconnect()| elif method == "scshell": | ||
| try: | ||
| exec_method = SCSHELL( | ||
| self.host if not self.kerberos else self.hostname + "." + self.domain, |
There was a problem hiding this comment.
This should be self.remoteName
|
Thanks a lot for the review - these are very fair points. For the restore concern: the current code already attempts to restore the original service configuration in a On #1174: I had the same impression while working on this. I am happy to adapt this in whichever direction fits the project best:
Regarding the service-name option: I agree that method-specific arguments should be treated carefully. The main reason I exposed it here is reliability, not just stealth. This technique depends more on the chosen service than methods like That said, I think a curated internal candidate list would be a good follow-up improvement if that is the preferred direction. If so, I can rework this PR to focus on:
|
|
Based on my understanding of pull request #1174, I would consider these to be two separate options, as 1174 still relies on the presence of PsExec, which this pull request does not use. The original SCShell (https://github.com/Mr-Un1k0d3r/SCShell) was designed as a truly fileless lateral movement technique, as it does not require dropping any files, unlike using PsExec as the execution channel. |
|
Okay yeah my idea was that maybe is #1174 is a specific version of the general idea here in this PR and we therefore should do #1174 as a module that uses this exec-method technique with psexec. Similar to the Regarding https://github.com/Pennyw0rth/NetExec/pull/1185/changes#r3033220153, we talked internally a bit about cmd/powershell and general command invocation and the TLDR was pretty much that we should rip this out of the execution methods and generalize it in the protocol itself, so that you directly have the option to invoke cmd/powershell or your own binary.
Yeah I think that would be good. If we can generalize this so that it works without having to enumerate/check why which service work on which host that would be great (if possible). |
|
Ayo, I wouldn't merge both psexec and scshell because it makes things harder for a user to know exactly what's happening. Also, I kinda expect psexec to be the default opsec option for command execution so... (eyes) Concerning the hardcoded cmd/powershell binary, I'll be looking at this after finishing some PR's of mine :)! |
|
Sounds good, then let's keep them separate👍 |
4 participants
Description
This PR adds a new SMB command execution method:
--exec-method scshell.The implementation uses SCMR to temporarily update the target service binary path, start the service to execute the supplied command, and then restore the original service configuration.
This adds an additional execution method for SMB
-x/-Xusage. The method currently does not support command output retrieval, so it is intended to be used with--no-output.Additional CLI options were added to support the technique:
--sc-service-nameto choose the target service--sc-no-cmdto avoid prependingcmd.exe /cTests were also added/updated:
--exec-method scshelltests/e2e_commands.txtupdated with SMB scshell examplesNo new third-party dependencies are required.
Relevant sources / references:
ChangeServiceConfigAI disclosure:
This PR was AI-assisted using OpenAI Codex. AI was used for implementation assistance and test scaffolding. All code was reviewed, adjusted, and manually tested by me before submission.
Type of change
Setup guide for the review
Tested locally on Linux using Python 3.13.
Validation performed locally:
pytest tests/test_smb_cli.py tests/test_smb_database.py -qpython -m py_compile nxc/protocols/smb/scshell.py nxc/protocols/smb.py nxc/protocols/smb/proto_args.py tests/test_smb_cli.pypipx install --editable . --suffix=-scshelltestExample review/test command:
netexec smb TARGET -u USER -p PASS -x whoami --exec-method scshell --sc-service-name RemoteRegistry --no-outputPowerShell variant:
netexec smb TARGET -u USER -p PASS -X whoami --exec-method scshell --sc-service-name RemoteRegistry --no-outputNotes for reviewers:
RemoteRegistrywas used as the default/configurable service name for testingScreenshots (if appropriate):
Checklist:
poetry run ruff check ., use--fixto automatically fix what it can)tests/e2e_commands.txtfile if necessary (new modules or features are required to be added to the e2e tests)