XOR encryption with c++

2
votes

I've implemented a fairly simple method to encrypt and decrypt string to a file and then back. The methods im using look as following:

string encrypt(string msg, string key) {
    string tmp(key);
    while (key.size() < msg.size()) key += tmp;
    for (std::string::size_type i = 0; i < msg.size(); ++i) msg[i] ^= key[i];
    return msg;
}

string decrypt(string msg, string key) {
    return encrypt(msg, key);
}

However some of the keys I am using are not very useful. The string gets encrypted correctly and written to a file. But when i try to decrypt everything (loading the file into a string, decrypt it, write is back to another file) the new file is significant smaller and doesnt contain the entire information stored in it.

The keys I've tried so far are:

string crypt = "abc";                           //works kinda well
string crypt = "_H84M!-juJHFXGT01X1*G6a$gEv";   //doesnt work
string crypt = "H84MjuJHFXGT01X1G6agEv";        //doesnt work either

I am hoping you can help me and give me any advice on how to choose a usable key.

The code for the file handling:

ofstream temp;
temp.open("temp", ios::in | ios::trunc);
temp << encrypt(buffer, crypt);
temp.close();


ifstream in(file);
string content((std::istreambuf_iterator<char>(in))                (std::istreambuf_iterator<char>()));
ofstream plain;
plain.open(newfile, ios::in | ios::trunc);
plain << decrypt(content, crypt);
plain.close();
3
c++encryptionxor
I expect you getting a binary 0 in your encrypted string and then use a C string function that expects a string terminated with a binary 0 char. This will truncate your encrypted data. – Richard Critten
Show your code for reading and writing to the file. – Nelfeal
Just use msg[i] ^= key[i%key.size()]; instead of nonsense with increasing key. – Slava
You should treat the encrypted data as a binary stream that may contain any values and you should not use character oriented functions (that may alter the actual data read/written like EOL conversions) to write and read that data – Serge
"...to jump over" - either you pick an algorithm that does not produce zeroes or you follow my recommendation in my previous comment. – Serge

3 Answers

2
votes

As you have binary data in your encrypted string you should use unformatted write method instead of operator<<:

ofstream os(...);
std::string encrypted = ...;
os.write( encrypted.data(), encrypted.size() );

Note you may want to write data size before actual data if you need more than one encrypted string in a file. Then you read data size and data with istream::read():

void write( std::ostream &out, const std::string &encrypted )
{
    size_t length = encrypted.size(); 
    of.write( &length, sizeof( length ) );
    of.write( encryped.data(), length );
}

std::string read( std::istream &in )
{
     size_t length = 0;
     in.read( &length, sizeof( length ) );
     std::string str( length );
     in.read( &str[0], length );
     return str;
}

Note 2: it could be a good idea to store encrypted data in std::vector<char> instead of std::string, that will prevent many problems - you will not be able to use many functions that implicitly assume that string is null-terminated.

2
votes

I just wrote a complete, minimal, working example.

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

static std::string str_xor(const std::string &data, const std::string &key) {
  std::string result(data.size(), '\0');

  for (std::size_t i = 0, i < data.size(); i++) {
    result[i] = data[i] ^ key[i % key.size()];
  }
  return result;
}

int main(int argc, char **argv) {
  if (argc != 3) {
    std::cerr << "usage: xor <datafile> <keyfile>\n";
    return 1;
  }

  std::ifstream data_in(argv[1]);
  std::string data(
    (std::istreambuf_iterator<char>(data_in)),
    (std::istreambuf_iterator<char>()));
  data_in.close();

  std::ifstream key_in(argv[2]);
  std::string key(
    (std::istreambuf_iterator<char>(key_in)),
    (std::istreambuf_iterator<char>()));
  key_in.close();

  std::string result = str_xor(data, key);

  std::ofstream data_out(argv[1]);
  data_out << result;
  data_out.close();

  return 0;
}

It is still missing the error checking for the files, just in case they are not found. But if you pass the names of two existing files to it, it works like a charm, encrypting the first file using the second file as a key.

Caveat: Don't use this program in practice, for the excellent reasons given in https://codereview.stackexchange.com/a/140366.

1
votes

Okay, all of your answers and comments pointed me in the right direction, but didnt work directly in my program.

First, I changed the encryption algorithm in the way Slava suggested. Second, I switched the file handling The new way gets the entire length of the encrypted file and forces each character via an array of char into a new string. I know this isnt pretty, but I am able keep the entire code in string type.

So I came up with the following:

ifstream in(file, ios::binary);
in.seekg(0, ios::end);              // go to the end
int length = in.tellg();            // report location (this is the length)
in.seekg(0, ios::beg);              // go back to the beginning
char* content = new char[length];   // allocate memory for a buffer of appropriate dimension
in.read(content, length);           // read the whole file into the buffer
string content2 = "";
for (int i = 0; i < length; i++) con31 += con3[i];  //append to string character by character
ofstream plain;
plain.open(newfile, ios::in | ios::trunc);
plain << decrypt(content2, crypt);
plain.close();

This works pretty well for me. I hope I havent build in some heavy mistakes.