-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Handle unknown NTSTATUS in SessionError #1311
Conversation
Hi @rtpt-lucasvater Thanks for your PR and sorry late response on this one I've been checking your changes and the only thing that kept me thinking is that perhaps we may also have that same issue in other parts of the lib Looking at all references to
Perhaps we can implement something similar there to keep it synchronized with the lib I've also found that this error could be triggered from other sources as well, where no check is being done
|
Hi @gabrielg5, Thanks for the response! I changed the mentioned places to handle the errors in a similar way. However, I currently can't test the changes, so I can't give any guarantees that there are no mistakes on my part. Perhaps you can take a look and see if the changes reflect what you had in mind. |
thanks @rtpt-lucasvater!! |
@rtpt-lucasvater this broke some stuff in NetExec (Pennyw0rth/NetExec#117), since you are returning a string of the entire error instead of a tuple from nt_errors now. |
Can confirm, it's a big breaking change, removing the hability to only have the error status is a downgrade imo |
@Marshall-Hallenbeck @mpgn To get the status string you could use the |
The whole point of In the future it'd be nice if breaking changes like this were tested a little more. |
* Handle unknown NTSTATUS in SessionError * Handle unknown NTSTATUS in other places
…getErrorString()" This reverts commit bcacac1.
…hange getErrorString for nt_status errors
…hange getErrorString for nt_status errors
…ring (#1714) * fix(nterrors): undo untested breaking changes from #1311 that change getErrorString for nt_status errors * fix(nt_errors): had incorrect older reference, updated to nt_errors * Update impacket/krb5/kerberosv5.py Co-authored-by: Gabriel Gonzalez <[email protected]> --------- Co-authored-by: Gabriel Gonzalez <[email protected]>
When a
SessionError
is raised during SMB session negotiation, currently unexpected NTSTATUS error codes are not handled properly. If theSessionError
exception is catched and logged, a KeyError is thrown when retrieving the string representation of the exception in:https://github.com/SecureAuthCorp/impacket/blob/d509775976ba37f4eaea630cc511e2fc3b65aba3/impacket/smb.py#L581
The error occurred when receiving an unexpected error code during an SMB login scan using
CrackMapExec
.This pull request adds a generic error message when the error code is not contained in the
ERROR_MESSAGES
dictionary.