SSH module is not iterating on the credential list properly#99
Open
schischi wants to merge 1 commit intonmap:masterfrom
schischi:ssh_bad_creds
Open
SSH module is not iterating on the credential list properly#99schischi wants to merge 1 commit intonmap:masterfrom schischi:ssh_bad_creds
schischi wants to merge 1 commit intonmap:masterfrom
schischi:ssh_bad_creds
Conversation
The logic to get the credentials for the SSH module is wrong: - Pairwise: the module is using the wrong pairs of credentials: user[idx] with pass[idx+6]; - Default: the first 6 passwords are ignored for all the usernames except the first one; With pairwise: $ ./ncrack -vvv -d10 --pairwise -U <(seq 100) -P <(seq 100) ssh://127.0.0.1 | grep Login (EID 1) Login failed: '1' '1' (EID 1) Login failed: '1' '2' << should be '2' '2' (EID 1) Login failed: '1' '3' << should be '3' '3' (EID 1) Login failed: '1' '4' << ... (EID 1) Login failed: '1' '5' << ... (EID 1) Login failed: '1' '6' << ... (EID 1) Login failed: '2' '7' << should be '7' '7' ... (EID 24) Login failed: '32' '38' ... (EID 83) Login failed: '81' '87' ... (EID 105) Login failed: '99' '5' (EID 106) Login failed: '100' '6' As we can see, ncrack is using the wrong pairs of credentials: user[idx] with pass[idx+6]. With default option: $ ./ncrack -vvv -d10 -U <(seq 3) -P <(seq 10) ssh://127.0.0.1 | grep Login (EID 1) Connection closed by peer (EID 1) Login failed: '1' '1' (EID 1) Login failed: '1' '2' (EID 1) Login failed: '1' '3' (EID 1) Login failed: '1' '4' (EID 1) Login failed: '1' '5' (EID 1) Login failed: '1' '6' (EID 2) Login failed: '1' '7' (EID 4) Login failed: '1' '8' (EID 6) Login failed: '1' '9' (EID 8) Login failed: '1' '10' (EID 3) Login failed: '2' '7' << missing data here (6 creds) (EID 5) Login failed: '2' '8' (EID 7) Login failed: '2' '9' (EID 9) Login failed: '2' '10' (EID 3) Login failed: '3' '7' << missing data here (6 creds) (EID 5) Login failed: '3' '8' (EID 7) Login failed: '3' '9' (EID 9) Login failed: '3' '10' Ncrack is skipping the first 6 passwords for all the users except the first one. Note that if we run the same command but with the mysql module, everything looks good: Example: $ ./ncrack -vvv -d10 --pairwise -U <(seq 100) -P <(seq 100) mysql://127.0.0.1 | grep Login mysql://127.0.0.1:3306 (EID 1) Login failed: '1' '1' mysql://127.0.0.1:3306 (EID 2) Login failed: '2' '2' ... mysql://127.0.0.1:3306 (EID 99) Login failed: '99' '99' mysql://127.0.0.1:3306 (EID 100) Login failed: '100' '100' The commit f2270de introduced this regression and reverting it fix the issue. The description of the commit lacks a bit of context, but I guess the intent was that if we are using the same usernames for different attempts, then we could keep the same connection open and speed up things during the first timing probe. Even if reverting this commits might cause a bit of performance drop, I think it's more important to have code that behave as it should. With this commit applied: $ ./ncrack -vvv -d10 --pairwise -U <(seq 20) -P <(seq 20) ssh://127.0.0.1 | grep Login (EID 1) Login failed: '1' '1' ... (EID 21) Login failed: '20' '20'
|
@ithilgore could this be merged in your opinion? It looks sensible as some credentials are not being used while bruteforcing! |
ithilgore
requested changes
Aug 4, 2021
Collaborator
ithilgore
left a comment
There was a problem hiding this comment.
Place inside #if 0 and #endif instead of deleting. Might have to fix this later on.
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.
The logic to get the credentials for the SSH module is wrong:
With pairwise:
As we can see, ncrack is using the wrong pairs of credentials: user[idx] with pass[idx+6].
With default option:
Ncrack is skipping the first 6 passwords for all the users except the first one.
Note that if we run the same command but with the mysql module, everything looks good:
Example:
The commit f2270de introduced this regression and reverting it fix the issue.
The description of the commit lacks a bit of context, but I guess the intent was that if we are using the same usernames for different attempts, then we could keep the same connection open and speed up things during the first timing probe.
Even if reverting this commits might cause a bit of performance drop, I think it's more important to have code that behave as it should.
With this commit applied: