patterncppModerate
Brute force password-cracker
Viewed 0 times
brutecrackerforcepassword
Problem
I am just coding some classic brute force password cracking program, just to improve myself.
I've explained how my program works at the start of the code. Check some of those screenshots to understand easier.
My program works really well but it's a bit dirty and it can be faster if I solve these two problems:
-
The main code is not that long. It looks so long and dirty because I copied a code block 8 times in a switch case statement. For example, case 1 loops with one character length passwords.
-
It tries 1 digit first, then 2 digits, then 3 digits and so on. So it should wait for 1, 2, 3 to get the 4 digit ones. And it makes the program lose so much time at higher digits. My CPU is i7 3770k got 6 cores and the program runs only with one. I guess it's because it says 13% CPU usage. I want to make it higher, like 6 cores on the same task, or each core will take care of one part. The first core will start looping only the 8 character length ones, and second core will do the same with 7 character length ones... and when one of them finds the answer, the program will end. Can we really do it?
Here is the code
```
#include
#include
using namespace std;
string crackPassword(string pass);
long long int attempt;
clock_t start_t, end_t;
int main(){
string password;
cout > password;
cout \n>> CRACKED THE PASSWORD! >>\n>" << endl << endl <<"The password : " << crackPassword(password) << endl;
cout << "The number of attempts : " << attempt << endl;
cout << "The time duration passed : " << (double)(end_t - start_t)/1000 << " seconds" << endl << endl;
return
I've explained how my program works at the start of the code. Check some of those screenshots to understand easier.
My program works really well but it's a bit dirty and it can be faster if I solve these two problems:
-
The main code is not that long. It looks so long and dirty because I copied a code block 8 times in a switch case statement. For example, case 1 loops with one character length passwords.
case 2 = two characters, case 8 = 8 characters length password. The one and only difference between those cases is the "for loop" count. case 1 has 1 for loop, case 8 has 8 nested for loops. I want to make my code prettier, so how can I get rid of this copy/pasted code and make it 1/8 size of current size? CTRL + MOUSE WHEEL DOWN, zoom-out and see the copy pasted parts.-
It tries 1 digit first, then 2 digits, then 3 digits and so on. So it should wait for 1, 2, 3 to get the 4 digit ones. And it makes the program lose so much time at higher digits. My CPU is i7 3770k got 6 cores and the program runs only with one. I guess it's because it says 13% CPU usage. I want to make it higher, like 6 cores on the same task, or each core will take care of one part. The first core will start looping only the 8 character length ones, and second core will do the same with 7 character length ones... and when one of them finds the answer, the program will end. Can we really do it?
Here is the code
```
#include
#include
using namespace std;
string crackPassword(string pass);
long long int attempt;
clock_t start_t, end_t;
int main(){
string password;
cout > password;
cout \n>> CRACKED THE PASSWORD! >>\n>" << endl << endl <<"The password : " << crackPassword(password) << endl;
cout << "The number of attempts : " << attempt << endl;
cout << "The time duration passed : " << (double)(end_t - start_t)/1000 << " seconds" << endl << endl;
return
Solution
-
Prefer not to get into the habit of using
-
Make sure to include `
If this is being avoided because the line is too long, then it should be effectively shortened in some other way. All of that will help with readability.
Prefer not to get into the habit of using
using namespace std.-
Make sure to include `
.
-
For the library, you should use std::clock_t instead of clock_t.
-
Prefer to avoid global variables:
long long int attempt;
clock_t start_t, end_t;
As these variables can be modified anywhere in the program, you could introduce bugs, which will also hurt maintainability and testability.
You should have attempt initialized to 0 (it's being incremented) in main() and pass it to crackPassword() by reference. In this way, you'll know that only these two functions can recognize attempt (if you ever add additional functions).
start_t and end_t just need to be in main(). I'd also recommend renaming them (especially remove the _t), otherwise it may look like they're part of the library.
-
There's no need to use std::endl so many times, which also flushes the buffer, needlessly slowing down the code. Just use "\n", which only gives a newline. It'll also make the code a bit shorter, especially in places where it can be put into an existing hard-coded output line.
This, for instance:
cout \n>> CRACKED THE PASSWORD! >>\n>" << endl << endl <<"The password : " << crackPassword(password) << endl;
would turn into this:
cout \n>> CRACKED THE PASSWORD! >>\n>\n\n The password : " << crackPassword(password) << "\n";
You could also split this into separate lines for clarity, and to keep the lines shorter:
cout \n>> CRACKED THE PASSWORD! >>\n>\n\n;
cout << "The password : " << crackPassword(password) << "\n";
-
You don't specifically need a C-style cast here:
(double)(end_t - start_t)/1000
Cast the C++ way, with static_cast<>:
static_cast(end_t - start_t)/1000
Also, in case you'll need to use this in other places, consider having it as a variable. You should also use the CLOCKS_PER_SEC macro, which is part of the library.
double timeDuration = static_cast(end_t - start_t) / CLOCKS_PER_SEC;
-
In crackPassword(), pass should be passed by const& instead of by value as it's not being modified inside the function. This will also save an unnecessary copy.
string crackPassword(string const& pass){
-
Prefer to have one variable declaration/initialization per line:
int digit[7];
int alphabetSet=1;
int passwordLength=1;
This will allow each variable to be more visible. It will also be possible to add a comment for separate variables if needed.
-
A line like this:
for(digit[0]=1;digit[0]<alphabet.length();digit[0]++){
should use appropriate whitespace for readability:
for (digit[0] = 1; digit[0] < alphabet.length(); digit[0]++) {
Generally, keep whitespace between operators, and in the case of for` loop statements, separate each part as well.If this is being avoided because the line is too long, then it should be effectively shortened in some other way. All of that will help with readability.
Code Snippets
long long int attempt;
clock_t start_t, end_t;cout << endl << endl << endl << ">\n>> CRACKED THE PASSWORD! >>\n>" << endl << endl <<"The password : " << crackPassword(password) << endl;cout << "\n\n\n>\n>> CRACKED THE PASSWORD! >>\n>\n\n The password : " << crackPassword(password) << "\n";cout << "\n\n\n>\n>> CRACKED THE PASSWORD! >>\n>\n\n;
cout << "The password : " << crackPassword(password) << "\n";(double)(end_t - start_t)/1000Context
StackExchange Code Review Q#44620, answer score: 16
Revisions (0)
No revisions yet.