Skip to content
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

Make some text more precise #7

Merged
merged 9 commits into from
Sep 14, 2023
Merged

Conversation

NeffIsBack
Copy link
Contributor

Change some help and fail messages to better bring across what is happening in the background

@NeffIsBack NeffIsBack added the documentation Improvements or additions to documentation label Sep 9, 2023
Copy link
Collaborator

@Marshall-Hallenbeck Marshall-Hallenbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few spots where you say "(maybe command executed!)" - why is this? Like, technically why - is it a high chance it executed? I'm not sure we should get the user's hopes up with a small chance at execution. Rather, maybe we should try to confirm execution?
I know a lot of places in the code are iffy about if something worked or not, maybe we should come up with a "maybe it worked, but we can't tell" error?

cme/protocols/smb/atexec.py Outdated Show resolved Hide resolved
cme/protocols/smb/mmcexec.py Outdated Show resolved Hide resolved
cme/protocols/smb/smbexec.py Outdated Show resolved Hide resolved
cme/protocols/wmi/wmiexec.py Outdated Show resolved Hide resolved
cme/protocols/wmi/wmiexec_event.py Outdated Show resolved Hide resolved
cme/protocols/smb/mmcexec.py Outdated Show resolved Hide resolved
cme/protocols/smb/atexec.py Outdated Show resolved Hide resolved
@NeffIsBack
Copy link
Contributor Author

There's a few spots where you say "(maybe command executed!)" - why is this? Like, technically why - is it a high chance it executed? I'm not sure we should get the user's hopes up with a small chance at execution. Rather, maybe we should try to confirm execution? I know a lot of places in the code are iffy about if something worked or not, maybe we should come up with a "maybe it worked, but we can't tell" error?

From my understanding this is because it tries to execute the code on the host, but sometimes fails too read the output properly (most often i encountered just a time out, but this can very well be an AV as well). Any idea how we could improve it?
Also probably something @XiaoliChan can help with

@XiaoliChan
Copy link
Contributor

There's a few spots where you say "(maybe command executed!)" - why is this? Like, technically why - is it a high chance it executed? I'm not sure we should get the user's hopes up with a small chance at execution. Rather, maybe we should try to confirm execution?
I know a lot of places in the code are iffy about if something worked or not, maybe we should come up with a "maybe it worked, but we can't tell" error?

Yes just like @NeffIsBack said, sometimes it got detection by AV software and you never know about it(the impacket has no error), I can 100% confirm these situation always happens

@Marshall-Hallenbeck
Copy link
Collaborator

There's a few spots where you say "(maybe command executed!)" - why is this? Like, technically why - is it a high chance it executed? I'm not sure we should get the user's hopes up with a small chance at execution. Rather, maybe we should try to confirm execution?
I know a lot of places in the code are iffy about if something worked or not, maybe we should come up with a "maybe it worked, but we can't tell" error?

Yes just like @NeffIsBack said, sometimes it got detection by AV software and you never know about it(the impacket has no error), I can 100% confirm these situation always happens

This is true about our SMB exec, etc - but we don't have this same alert for users. I think if there's an error that we don't know about, and we can't be at least 90% sure that it executed, we should just return the network error. Does that make sense?

@XiaoliChan
Copy link
Contributor

XiaoliChan commented Sep 10, 2023

This is true about our SMB exec, etc - but we don't have this same alert for users. I think if there's an error that we don't know about, and we can't be at least 90% sure that it executed, we should just return the network error. Does that make sense?

Yes, it will, if get other types of exception, it will output the error log in debug log
https://github.com/Pennyw0rth/CrackMapExec/blob/master/cme/protocols/smb/smbexec.py#L183

@NeffIsBack
Copy link
Contributor Author

Yes, it will, if get other types of exception, it will output the error log in debug log https://github.com/Pennyw0rth/CrackMapExec/blob/master/cme/protocols/smb/smbexec.py#L183

Do you know what happens when AV catches the exec method? Does that trigger the "tries" exception with just the file missing or is this one of the other exceptions then? If this triggers one of the other exec methods we could just inform, that the command probably executed, but the timeout was just too short

@XiaoliChan
Copy link
Contributor

XiaoliChan commented Sep 10, 2023

Do you know what happens when AV catches the exec method?

AV have many many way to catch it, like block RPC access, remove the results files

Does that trigger the "tries" exception with just the file missing or is this one of the other exceptions then? If this triggers one of the other exec methods we could just inform, that the command probably executed, but the timeout was just too short

It always raises "STATUS_OBJECT_NAME_NOT_FOUND" when can't find the result files, I can confirm this.

@NeffIsBack
Copy link
Contributor Author

Does that trigger the "tries" exception with just the file missing or is this one of the other exceptions then? If this triggers one of the other exec methods we could just inform, that the command probably executed, but the timeout was just too short

It always raises "STATUS_OBJECT_NAME_NOT_FOUND" when can't find the result files, I can confirm this.

Okay, then it nearly always will be a simple timeout and AV will result in another exception correct? I will write that more explicit then to be clear that it most likely did execute

@XiaoliChan
Copy link
Contributor

XiaoliChan commented Sep 10, 2023

Okay, then it nearly always will be a simple timeout and AV will result in another exception correct? I will write that more explicit then to be clear that it most likely did execute

Yes, sometimes is like if you executed the command with result output, and the command has executed, but the AV remove the result file, in this case, the command was executed

But I never meet the something like "connection timeout" before

@NeffIsBack
Copy link
Contributor Author

Hmm okay, for me i had several times the problem, that the command executed but i set the threshhold too low. So it executed but didn't show the output because it tried to grab the file too fast.

@@ -38,6 +38,8 @@ def __init__(
self.__doKerberos = doKerberos
self.__kdcHost = kdcHost
self.__tries = tries
self.__output = None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably rename this "__output_filename"

@@ -114,11 +116,12 @@ def gen_xml(self, command, tmpFileName, fileless=False):
<Command>cmd.exe</Command>
"""
if self.__retOutput:
self.__output = "\\Windows\\Temp\\" + gen_random_string(6)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want the folder to be a variable, so users can pass in their own temp location?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be another feature. Probably great to add but should be done seperatly imo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want the folder to be a variable, so users can pass in their own temp location?

Yes, as @NeffIsBack said, this is another feature

break
except Exception as e:
if tries >= self.__tries:
self.logger.fail(f'ATEXEC: Get output file error, maybe got detected by AV software, please increase the number of tries with the option "--get-output-tries". If it\'s still failing maybe something is blocking the schedule job, try another exec method')
self.logger.fail(f"ATEXEC: Couldn't retrieve output file, maybe got detected by AV. Please increase the number of tries with the option '--get-output-tries'. If it's still failing, try the wmi protocol or another exec method")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ATEXEC: Could not retrieve output file, it may have been detected by AV. Please increase the number of tries with the option '--get-output-tries'. If it is still failing, try the 'wmi' protocol or another exec method

break
if str(e).find("STATUS_BAD_NETWORK_NAME") >0 :
self.logger.fail(f'ATEXEC: Get ouput failed, target has blocked ADMIN$ access (maybe command executed!)')
self.logger.fail(f"ATEXEC: Get output failed, target has blocked {self.__share} access (maybe command executed!)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ATEXEC: Getting the output file failed - target has blocked access to the share: {self.__share} (but the command may have executed!)

@@ -252,10 +252,10 @@ def get_output_remote(self):
break
except Exception as e:
if tries >= self.__tries:
self.logger.fail(f'MMCEXEC: Get output file error, maybe got detected by AV software, please increase the number of tries with the option "--get-output-tries". If it\'s still failing maybe something is blocking the schedule job, try another exec method')
self.logger.fail(f"MMCEXEC: Couldn't retrieve output file, maybe got detected by AV. Please increase the number of tries with the option '--get-output-tries'. If it's still failing, try the wmi protocol or another exec method")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MMCEXEC: Could not retrieve output file, it may have been detected by AV. Please increase the number of tries with the option '--get-output-tries'. If it is still failing, try the 'wmi' protocol or another exec method

break
if str(e).find("STATUS_BAD_NETWORK_NAME") >0 :
self.logger.fail(f'MMCEXEC: Get ouput failed, target has blocked {self.__share} access (maybe command executed!)')
self.logger.fail(f"MMCEXEC: Get output failed, target has blocked {self.__share} access (maybe command executed!)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MMCEXEC: Getting the output file failed - target has blocked access to the share: {self.__share} (but the command may have executed!)

break
if str(e).find("STATUS_BAD_NETWORK_NAME") >0 :
self.logger.fail(f'SMBEXEC: Get ouput failed, target has blocked {self.__share} access (maybe command executed!)')
self.logger.fail(f"SMBEXEC: Get output failed, target has blocked {self.__share} access (maybe command executed!)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SMBEXEC: Getting the output file failed - target has blocked access to the share: {self.__share} (but the command may have executed!)

@@ -170,10 +170,10 @@ def get_output_remote(self):
break
except Exception as e:
if tries >= self.__tries:
self.logger.fail(f'SMBEXEC: Get output file error, maybe got detected by AV software, please increase the number of tries with the option "--get-output-tries". If it\'s still failing maybe something is blocking the schedule job, try another exec method')
self.logger.fail(f"SMBEXEC: Couldn't retrieve output file, maybe got detected by AV. Please increase the number of tries with the option '--get-output-tries'. If it's still failing, try the wmi protocol or another exec method")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SMBEXEC: Could not retrieve output file, it may have been detected by AV. Please increase the number of tries with the option '--get-output-tries'. If it is still failing, try the 'wmi' protocol or another exec method

Copy link
Collaborator

@Marshall-Hallenbeck Marshall-Hallenbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Marshall-Hallenbeck Marshall-Hallenbeck merged commit 95fcb55 into main Sep 14, 2023
@NeffIsBack NeffIsBack deleted the neff-text-improvements branch September 14, 2023 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants