7
\$\begingroup\$

I've written a CLI tool in Python, for bruteforcing hashes (only md5 with this version). Apart from the fact that Python is not exactly suitable for bruteforcing (performance), what else can I do to improve my code (performance -otherwise-, styling, naming, argument handling etc.)?

from sys import argv, exit
from time import time
from hashlib import md5
from itertools import product
from datetime import datetime
from string import ascii_lowercase, ascii_uppercase, digits

hash_ = argv[1]
charset_ = argv[2]
minlength = argv[3]
maxlength = argv[4]

charset = {"1":  ascii_lowercase,
           "2":  ascii_uppercase,
           "3":  digits,
           "4":  ascii_lowercase 
               + ascii_uppercase,
           "5":  ascii_lowercase 
               + digits,
           "6":  ascii_uppercase
              + digits,
           "7":  ascii_lowercase 
              + ascii_uppercase 
              + digits
         }

 charset = charset[charset_]

 colors = {"R":"\033[91m",
           "G":"\033[92m",
           "~":"\033[00m"
         }


def bruteforce(hash_, characters, min_length, max_length):

    start = datetime.now()
    start_time = time()

    for length in range(int(min_length), int(max_length) + 1):
        products = product(characters, repeat=length)
        for attempt in products:
            hashed = "".join(attempt).encode("utf-8")
            hashed = md5(hashed).hexdigest()

            if hashed != hash_:
                print(f"{colors['R']}{''.join(attempt)}")

            else:
                diff = int(time()-start_time)
                print(f"{colors['G']}{''.join(attempt)}")
                print(colors["~"])
                print(" Statistics")
                print("~~~~~~~~~~~~~~~~~")
                print(f"Started:          {start}")
                print(f"Calculation time: {diff} seconds")
                print(f"Original hash:    {hash_}")
                print(f"Found string:     {''.join(attempt)}")
                print("~~~~~~~~~~~~~~~~~")

               return True


def main():
    print(colors["~"])
    bruteforce(hash_, charset, minlength, maxlength)
    print(colors["~"])

if __name__ == "__main__":
    main()
    exit()
\$\endgroup\$
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Simon Forsberg Jun 3 '17 at 9:11
3
\$\begingroup\$

You should put your argument processing also in your main function. The way it currently is, if I imported your function from another script, I would also have to have four command line arguments in that script, otherwise this will raise an IndexError.

You could also use tuple unpacking for this:

def main():
    hash_, charset_, minlength, maxlength = argv[1:5]
    print(colors["~"])
    bruteforce(hash_, charset[charset_], minlength, maxlength)
    print(colors["~"])

You should also strive to make your functions as modular as possible. If I wanted more performance from your Algorithm, the first thing I would do is get rid of the debug output telling me of all the failed attempts. This is not possible currently, without rewriting the function. The same goes for the timing output, this is a separate concern from bruteforcing a hash, so it should also be its own function. In summary, I would make bruteforce shorter and give it a debug argment so that it only prints the failed attempts if in debug mode and have a decorator which provides the timing output:

def timeit(func):
    def wrapper(*args, **kwargs):
        start = datetime.now().time()
        ret = func(*args, **kwargs)
        diff = int(datetime.now.time()-start_time)
        print(colors["~"])
        print(" Statistics")
        print("_________________________________________")
        print(f"""Started at: {start} Calculation time: {diff} seconds""")
        print("_________________________________________")
        return ret
    return wrapper


@timeit
def bruteforce(hash_, characters, min_length, max_length, debug=False):
    for length in range(int(min_length), int(max_length) + 1):
        for attempt in product(characters, repeat=length):
            attempt = "".join(attempt).encode("utf-8")
            hashed = md5(attempt).hexdigest()
            if hashed != hash_:
                if debug:
                    print(f"{colors['R']}{attempt}{colors['~']}")
            else:
                if debug:
                    print(f"{colors['G']}{attempt}{colors['~']}")  
                    print("Original hash: {hash_} Found string: {attempt}")
                return attempt

Note that by adding {colors['~']} to the prints, you don't need to reset the colors after your function anymore (you should always strive to put everything back after you messed it up).

\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer! Would you mind explaining why *args and **kwargs are preferred over 4 predefined arguments? If I understand correctly, using *args and **kwargs allows providing an arbitrary amount of arguments, but that seems irrelevant here?). Also, please explain this ret = func(*args, **kwargs)... What is ret? Why are you returning it and then returning the wrapper? It really confuses me :P \$\endgroup\$ – Daniel Jun 3 '17 at 22:37
  • 1
    \$\begingroup\$ @Coal_ First you should read that link about decorators. I chose to use *args, **kwargs here to make the decorator reusable. You can use it with any function accepting any number of arguments and keyword arguments. ret holds the return value of the decorated function. I first print the timing statistics and then​ return whatever the function returned. \$\endgroup\$ – Graipher Jun 3 '17 at 23:17
4
\$\begingroup\$

1) You don't need to pass minlength and maxlength in your bruteforce function since you making it available at module level.

2) Also there are unwanted new-lines.

3) Also dict formatting can be improved a lot, this will help in readability of your code. Try using any online dict-formatter.

4) You can use datetime.now().time() instead of using a separate library time.

5) If possible avoid using too many print statements or atleast do that with proper messaging rather than any random string like ~~~~~~ you are using.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your response. Can you explain what you mean by 2)? I'm not sure how to improve whitespace since it would only make the code less readable. \$\endgroup\$ – Daniel Jun 3 '17 at 7:51
  • 1
    \$\begingroup\$ Like in your function bruteforce after your definition starts there is a new line and then this start = datetime.now(), so that new-line is not required. Actually that's a personal choice. It's totally upto you. Only reason I was saying that was, I got this advice for the same. \$\endgroup\$ – Nikhil Rajawat Jun 3 '17 at 8:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.