5
\$\begingroup\$

I am currently learning C++, so I made this basic password wallet for Ubuntu.

Its source code is in the /opt directory and it has a symlink so i can launch it from any directory using pw in the terminal.

Here is what it does:
It asks for a password (pass) and a site (which is related to the pass). It decrypts the file (test.enc) that contains the passwords to another file (text.txt), writes the pass and site to test.txt, encrypt it back to test.enc and delete it (test.txt).

#include <iostream>
#include <fstream>
#include <string>

using namespace std;

int main(){
        char pass[30];
        string site;
        getline(cin, site);
        cin >> pass;
        system("cd /opt/PW/ && openssl enc -aes-256-ecb -d -iter 100000 -in test.enc -out test.txt");
        ofstream MyFile("/opt/PW/test.txt", ios::app);
        MyFile << pass << ":" << site << endl;
        MyFile.close();
        system("cd /opt/PW/ && openssl enc -aes-256-ecb -iter 100000 -in test.txt -out test.enc && rm -rf test.txt");
}

It's for personal use only.
I am curious to know how it can be exploited.

\$\endgroup\$
3
  • 3
    \$\begingroup\$ This looks very similar to your earlier, closed question. The right thing to do is to edit that question, which will automatically nominate it for re-opening - you don't need to post a new question. (Just for future reference - don't mess with it now; we can answer this question and delete the closed one). \$\endgroup\$ Mar 12, 2021 at 15:28
  • \$\begingroup\$ Not related to the code itself but if you're looking for a secure command line password manager, you might be interested in pass . \$\endgroup\$
    – Erwan
    Mar 13, 2021 at 23:20
  • \$\begingroup\$ There is one rudimentary vulnerability in the process - if you are saving the decrypted file somewhere, there is a window (however small) for a malicious program to open and lock the file, gaining access to your plaintext password and associated site \$\endgroup\$
    – Luke
    Mar 14, 2021 at 11:14

2 Answers 2

10
\$\begingroup\$

There's quite a few beginner problems here:

using namespace std;

This is a really bad idea. It means we no longer have any control over the names in the global namespace as the Standard Library evolves. If this is a habit you have, it's wise to escape it as soon as possible.

    char pass[30];
    string site;
    getline(cin, site);
    cin >> pass;

We have no idea whether any of this was successful, because we never checked the status of std::cin afterwards.

    system("cd /opt/PW/ && openssl enc -aes-256-ecb -d -iter 100000 -in test.enc -out test.txt");

Don't ignore the return value from system()! How do we know whether the command was successful?

And there are no measures preventing an adversary leaving a symlink in that location, thus redirecting your output to a file of their choosing.

    ofstream MyFile("/opt/PW/test.txt", ios::app);
    MyFile << pass << ":" << site << endl;

We haven't taken measures to set the appropriate permissions on the file, so it's entirely dependent on the process umask value whether others can read it. That's very lax.

Both of these problems would have been avoided if we'd used the mktemp() library function to give us a unique temporary named file.

    MyFile.close();

There's no point calling close() if we're going to ignore its return value! Again, this is important, because there are many factors that can prevent writing a file.

    system("cd /opt/PW/ && openssl enc -aes-256-ecb -iter 100000 -in test.txt -out test.enc && rm -rf test.txt");

Another system() invocation that should have its return value tested.

Why do we not remove the file if the command was unsuccessful? I don't think it's good to leave it lying around when that happens.


For the larger view, I think a shell would be a better choice than C++ for writing a program such as this, since it's mostly about coordinating other processes. I don't think we'd need any temporary files at all if we used pipes appropriately to pass data from one command to another, entirely eliminating the file-handling problems we have.

\$\endgroup\$
12
\$\begingroup\$

Avoid calling system()

If possible, avoid calling system(). There are almost always issues doing this:

  • It is slow: system() will spawn another shell, which has to parse the command line you give it, and then it will spawn the actual commands you want to run).
  • Constructing a command line itself has all kinds of issues. For example, are you sure openssl is in your $PATH? And what if the search path finds a different version of openssl than the one you expected (/usr/bin/openssl)?
  • There is no easy way to send and receive data to the commands you start, so you have to redirect the input/output to files.

Especially the latter issue is important: what if someone reads /opt/PW/test.txt between the two calls to system()?

It would be much better to use a crypto library. OpenSSL is not just a program, it's also a library that you can call directly from C. However, I would say it is not the easiest to work with. There are other libraries out there that make encrypting and decrypting files easier and safer, for example NaCL.

Cryptography is hard to get right

Trying to learn about cryptography is great! However, be aware that it is not easy to get cryptography right. You are using AES-256 in ECB mode, which is bad. Furthermore, you are not using any form of message authentication, so if someone tampers with the encrypted file, then even if they cannot decrypt it properly, when you try to decrypt it you will not get the same data back as you put it. Sometimes it will be obviously garbage, but it sometimes is possible to change the data in more subtle ways (especially with ECB mode, but other modes also have issues if not combined with a MAC).

Go ahead and try to improve your program, learn more about cryptography, but I strongly suggest you do not use your cryptographic code for anything serious until you get a much better understanding of cryptography.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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