Garbage at end of file after encryption and decryption

Question

I am trying to encrypt and decrypt a file (a text or whatsoever), so I decided to use the Crypto++. Below is my code.

crypt.h:

#ifndef CRYPT_HPP_
# define CRYPT_HPP_

# include <crypto++/aes.h>
# include <crypto++/osrng.h>
# include <crypto++/blowfish.h>
# include <crypto++/eax.h>
# include <crypto++/files.h>
# include <iostream>

using namespace CryptoPP;
using namespace std;

class Crypt
{
  public:
    Crypt() {};
    ~Crypt() {};

    int init();
    int encrypt(const string &file);
    int decrypt(const string &file, int const);

    AutoSeededRandomPool  _randomGenerator;
    SecByteBlock          _aesKey, _aesIV;
};

#endif /* !CRYPT_HPP_ */

crypt.cpp:

int Crypt::init()
{
    try
    {
        _aesKey.New(Blowfish::DEFAULT_KEYLENGTH);
        _aesIV.New(Blowfish::BLOCKSIZE);

        _randomGenerator.GenerateBlock(_aesKey, _aesKey.size());
        _randomGenerator.GenerateBlock(_aesIV, _aesIV.size());
    }
    catch (CryptoPP::Exception &e)
    {
        cerr << e.what() << endl;
        return (-1);
    }
    return (0);
}

The encryption and decryption is performed with:

int Crypt::encrypt(const string &fileToEncrypt)
{
    EAX< Blowfish >::Encryption e1;
    e1.SetKeyWithIV(_aesKey, _aesKey.size(), _aesIV);

    string encryptedFile = "crypt.txt";
    FileSource fs1(fileToEncrypt.c_str(), true,
                   new AuthenticatedEncryptionFilter(e1,
                        new FileSink(encryptedFile.c_str())));
    return (0);
}

int Crypt::decrypt(const string &fileToDecrypt)
{
    EAX< Blowfish >::Decryption e1;
    e1.SetKeyWithIV(_aesKey, _aesKey.size(), _aesIV);

    string finalFile = "decrypt.txt";
    FileSource fs1(fileToDecrypt.c_str(), true,
                   new AuthenticatedEncryptionFilter(e1,
                        new FileSink(finalFile.c_str())));
    return (0);
}

The problem is that when I decrypt and get the final file, I actually get the right output PLUS some weird binary stuff. Like this (encrypt then decrypt a Makefile, I skipped to the end as this is the most interesting part) :

 fclean: clean
     $(RM) $(NAME)
     $(RM) $(TEST)
     $(RM) -R $(OBJDIR)

 re: fclean all
 �ٌ[�MT̨z���,�o%

Did someone already face this problem ? Or can someone help me please ?

Thank you !

Welcome to Stack Overflow! Please edit your question to contain a [mcve] — Slava, Oct 11 '16 at 21:49
You might want to checkout the [Crypto++ wiki](http://cryptopp.com/wiki/Main_Page). It has lots of code examples. — jww, Oct 11 '16 at 21:59
Thank you. I just edited it :). I actually checked the crypto++ wiki. I just followed an example from there : https://www.cryptopp.com/wiki/Blowfish and replaced StringSource by FileSource. Actually it works but I can't find anyone having the end of file problem though... and I cannot see any problem with my code either — NPanda, Oct 11 '16 at 22:04
padding: https://en.wikipedia.org/wiki/Padding_(cryptography) to rescue — Serge, Oct 11 '16 at 22:11
@Serge - I don't believe EAX mode suffers the traditional padding *faux pas*. If its coming from usage, I'm guessing it would be part of the authentication tag since it appears to be about 12 bytes. — jww, Oct 11 '16 at 22:19
@jww I meant the initial problem: as I got, initially the OP had just block cipher applied to a string. I am not an expert in a libcrypto++ API specifically, I just pointed the source of a possible problem to the OP — Serge, Oct 11 '16 at 22:22
@Nyrii - In your case, I think the controlling document or article is [EAX Mode](https://cryptopp.com/wiki/Eax_mode). I updated the `init` in your code. I believe you were using a 0-sized IV, and then calling `SetKeyWithIV` which expected an IV of size `Blowfish::BLOCKSIZE`. If its not that, then look towards the garbage exists before the encryption operation. — jww, Oct 11 '16 at 22:25
Using a block cipher with a small block size (Blowfish has only 64 bit) together with a streaming mode like EAX is *really insecure*. Such a small block size could potentially permit the attacker to collect the complete code book. You really should use block ciphers with larger block sizes like AES. — Artjom B., Oct 12 '16 at 05:30
@jww I looked over the crypted file and the garbage is inexistant. I modified my code but the problem is still here. I'm trying to trick to get a correct file. (Cannot mention more than one user but to Artjom B. : Ok I understood ! Thanks.) — NPanda, Oct 12 '16 at 09:48
Don't use Blowfish, use AES. Even Blowfish's creator Bruce Schneier uses AES, not Blowfish or even Twofish. If you need authenticated encryption either use an authenticated AES mode such as GCM of do your own authentication. — zaph, Oct 12 '16 at 12:33
What does the extra garbage look like in hex? Two possibilities are cryptographic padding or missing an EoF marker somewhere. Looking at it in hex might help to see where the garbage came from. — rossum, Oct 12 '16 at 14:06

score -1 · Answer 1 · answered Oct 12 '16 at 11:20

So... I found a solution but this is NOT a real valuable one. I only found a little trick to do the job.

Requirement : You need the size of the file BEFORE it is encrypted.

Here is the decrypt method I modified :

 int                     Crypt::decrypt(const string &fileToDecryptName, const string &finalFileName,
                                      int const previousSize) {
  EAX<AES>::Decryption  decryption;
  ofstream              finalFile;
  ifstream              tmpFile;
  std::vector<char>     buffer(previousSize);

  try {
    decryption.SetKeyWithIV(_aesKey, _aesKey.size(), _aesIV);
    FileSource fs1(fileToDecryptName.c_str(), true,
              new AuthenticatedEncryptionFilter(decryption, new FileSink("tmp")));
  } catch (Exception &e) {
    std::cerr << e.what() << std::endl;
    return (-1);
  }

  finalFile.open(finalFileName.c_str());
  tmpFile.open("tmp");
  tmpFile.seekg(0, std::ios::beg);
  if (tmpFile.is_open() && tmpFile.read(buffer.data(), previousSize)) {
    if (finalFile.is_open()) {
      finalFile.write(buffer.data(), previousSize);
    } else {
      finalFile.close();
      tmpFile.close();
      throw CommonError("Error : the destination file cannot receive the decrypted content.");
    }
  } else {
    finalFile.close();
    tmpFile.close();
    throw CommonError("Error : decryption cannot be done due to an unexisting or empty file.");
  }
  finalFile.close();
  tmpFile.close();
  return (0);
}

TL;DR : I put the decrypted content into a temporary file then I opened it, read its content and copied the data (finalFile.write(buffer.data(), previousSize) actually copy the data until it reaches previousSize characters) into the final file. This is how you can get rid of the garbage at the end of the file.

It is not necessary to know the file's pre-encryption size, this is just covering an error in the use of the encryption primitives and that may well cause future problems. It is necessary to fully understand the problem, error and correct usage. — zaph, Oct 12 '16 at 12:29
Are you encrypting/decrypting a file in-place? If so, then you should visit questions like [Resizing a file in C++](http://stackoverflow.com/q/17182147) and [Truncate or resize a file in order to modify its end](http://stackoverflow.com/q/15154263). — jww, Oct 12 '16 at 16:38

Garbage at end of file after encryption and decryption

1 Answers1