I have been reading about passwords and hashing algorithms and what not and decided to write a program.

Overview: The user should be prompted to create a password the first time the program is executed. They should enter a key and confirm it. If they have executed the program previously, then they should just enter the password to gain access.

I determine if the user has run the program by checking if key.txt exists. Is there a more preferred method?

I tried to streamline some code with the two bool functions. Any other suggestions for cleaner or more concise code?

#include <iostream>

#include <fstream>

#include <string>

#include "sha256.h"

using namespace std;



bool keyExists() {

    bool keyExists = false;

    ifstream inFile("key.txt");



    if (inFile) {

        keyExists = true;

    }



    return keyExists;

}



bool isMatch(string key, string confirmKey) {



    bool match = false;

    if (key == confirmKey) {

        match = true;

    }



    return match;

}



int main() {



    if (keyExists()) {

        string key;

        string storedKey;

        cout << "Please enter key: ";

        getline(cin, key);



        SHA256 sha256;

        ifstream inFile("key.txt");

        getline(inFile, storedKey);



        if (isMatch(sha256(key), storedKey)) {

            cout << "Acces Granted!n";

        }

        else {

            cout << "Access Denied!n";

        }

    }



    else {

        string key;

        string confirmKey;



        cout << "Please create a key: ";

        getline(cin, key);

        cout << "Confirm key: ";

        getline(cin, confirmKey);



        if (isMatch(key, confirmKey)) {



            SHA256 sha256;

            ofstream outFile("key.txt");

            outFile << sha256(key);

        }



        else {

            cout << "Keys do not match!n";

        }

    }



    return 0;

}

Many thanks to Stephan Brumme for the awesome hashing algorithm code! This was very easy to implement.

1. Using namespace std isn't a good practice in header files, where your program should at least partially reside, because you introduce potential name conflicts. So don't do that, and prefix the names you're importing with std::, you'll get used to it in no time.




2. Your function keyExists can be improved upon. First, its name is not the best you could have found, because it suggests you're looking for the existence of a particular key, whereas what you really want to know if the program has been launched before. So is_first_use, or is_first_visit as an analogy to a website, would tell more. Besides, it doesn't have to be that verbose:

Original version

bool keyExists() {

    bool keyExists = false; // the homonymy is a bad idea

    ifstream inFile("key.txt");



    if (inFile) { 

        keyExists = true; // 

    }



    return keyExists;

}

first trimming

bool keyExists() {

    bool keyExists = false;

    ifstream inFile("key.txt");



    keyExists = inFile; // because streams can be converted to bool as you know



    return keyExists;

}

second trimming

bool keyExists() {

    ifstream inFile("key.txt");

    return inFile; // you specified the return type, so the conversion will occur

}

third trimming

bool keyExists() {

    return ifstream("key.txt"); // no need for a named object

}

So do you really need a function? It depends. If you want to let your program evolve from here, it's a good idea to keep the function, and implement a more viable test at a later point without disturbing the rest of your code. Otherwise it isn't worth the burden of finding a name for it.

3. Your function isMatch suffers from the same issue.

original version

bool isMatch(string key, string confirmKey) {



    bool match = false;

    if (key == confirmKey) {

        match = true;

    }



    return match;

}

suggested implementation

bool isMatch(const string& key, const string& confirmKey) { // pass it by reference to avoid a potentially costly copy

    return key == confirmKey; // no need to store that in a `bool` before returning it

}

Then again I'm not sure it needs a function of its own...

4. What's a valid key?

It might be a design choice, but it's very rare to allow for tabs, spaces and such inside a password. Is this really what you want? Is it compatible with every way to store passwords that you can think of? How would you constrain the choice of a password?

1.

Do not use

using namespace std;  

Instead you can use

using std::cout;  

using std::getline();

using std::ifstream;

using std::ofstream;

using std::cin;

using std::string;  

Here is a link to why we should avoid using namespace <name>; type of statements

2.

In the function bool keyExists(); your bool variable is not necessary you can directly return the output of expression inside if condition. Same can be done in the other bool function, if you worry about readability then you may add a comment line after the return statement stating the intent behind the return value.

Also you can define both bool functions as inline.

Another thing to keep in mind when doing security focused applications is reverse engineering.

Using that, a potential attacker can simply analyze your compiled program and then binary patch it to circumvent any security measures you have put in place.
Of course in your case this is rather trivial because the source code is available. However even without the original source it can be possible to do so for example by looking for certain tell-tale strings.

Running your program gives some great hints about program flow via the strings Access Denied! and Access Granted! respectively. If one were to analyze your program it would be easy to find references to above strings to pinpoint the location in your code that needs to be patched.

Ultimately your program comes down to this part:

if (isMatch(sha256(key), storedKey)) {

    cout << "Acces Granted!n";

}

else {

    cout << "Access Denied!n";

}

Which when disassembled could look something like this:

Notice how you can see part of your if statement, namely the condition test and then a jump (JNZ) as well as the message Access Denied!. if you follow the jump you will get to this part:

Which just happens to be the "protected" part of your program. The attacker now only has to "patch" the jump to always jump to the right location of your program regardless of user input.

In order to protect against this you can read up on obfuscating programs and making them harder to "crack".