Gargage redundancy character at the end of original text when decrypt using Crypto++

Question

I am using Crypto++, CTR mode, to encrypt and decrypt text in C++. Everything seem to worked 99%. Ecrypting success, decrypting is give back the original text too, but I given some extra random garbage redundancy text like 'ð', at the end of the decrypted text. This extra part is random generated each time I run the code. Is there something wrong in my code?

Encrypt a string to a string

string  encryptString(string plain, byte key[], int sizeKey, byte iv[], int sizeIV){
    string cipher;
    try{
        CTR_Mode< AES >::Encryption e;
        e.SetKeyWithIV(key, sizeKey, iv, sizeIV);

        // The StreamTransformationFilter removes
        //  padding as required.
        StringSource s(plain, true,
            new StreamTransformationFilter(e,
                new StringSink(cipher)
                ) 
            ); 

#if 0
        StreamTransformationFilter filter(e);
        filter.Put((const byte*)plain.data(), plain.size());
        filter.MessageEnd();

        const size_t ret = filter.MaxRetrievable();
        cipher.resize(ret);
        filter.Get((byte*)cipher.data(), cipher.size());
#endif
        return cipher;
    }
    catch (const CryptoPP::Exception& e)
    {
        cerr << e.what() << endl;
        return NULL;
    }
}

Decrypt a ciphered string to a string

string  decryptString(string cipher, byte key[], int sizeKey, byte iv[], int sizeIV){
    string reco ="";
    try{
        CTR_Mode< AES >::Decryption d;
        d.SetKeyWithIV(key, sizeKey, iv, sizeIV);

        StringSource s(cipher, true,
            new StreamTransformationFilter(d,
                new StringSink(reco)
                ) 
            ); 

    }
    catch (const CryptoPP::Exception& e)
    {
        cerr << e.what() << endl;
    }
    return reco;

}

Wrap encryptString function above.

char* encrypt(char * plainText, byte key[], int sizeKey, byte iv[], int sizeIV, long &len){
    string cipher = encryptString(plainText, key, sizeKey, iv, sizeIV);
    len = cipher.size() + 1;
    char * writable = new  char[len];
    std::copy(cipher.begin(), cipher.end(), writable);
    writable[len] = '\0'; // don't forget the terminating 0
    return writable;
 }

Wrap decryptString function above.

char* decrypt(char * cipher,  byte key[], int sizeKey, byte iv[], int sizeIV, long len){
    string ss(cipher, len);
    long lengSS = ss.length();
    string recovered = decryptString(ss, key, sizeKey, iv, sizeIV);
    char * writable = new char[recovered.size() + 1];
    std::copy(recovered.begin(), recovered.end(), writable);
    writable[recovered.size()] = '\0'; // don't forget the terminating 0
    return writable;
}

My test script is simple. Read the some.txt content ("I love you"), write it to s1.txt to check if the reading is right. Encrypt, decrypt, then write the recovered text to another file (d1.txt).

int main(int argc, char* argv[])
{
    AutoSeededRandomPool prng;

    byte key[AES::DEFAULT_KEYLENGTH] = { '1', '2', '3', '4', '5', '6', '7', '8', '1', '2', '3', '4', '5', '6', '7', '8' };
    //prng.GenerateBlock(key, sizeof(key));

    byte iv[AES::BLOCKSIZE] = { '8', '7', '6', '5', '4', '3', '2', '1', '8', '7', '6', '5', '4', '3', '2', '1' };
    prng.GenerateBlock(iv, sizeof(iv));
    long size = 0;
    char * s1 = FileUtil::readAllByte("some.txt");
    //Result: s1.txt content is "I love you"

    long len = 0;
    char* result1 = encrypt(s1, key, sizeof(key), iv, sizeof(iv), len);
    //Result: result1 is a bunch of ciphered characters

    cout << "desc" << endl;
    char* recovered1 = decrypt(result1, key, sizeof(key), iv, sizeof(iv), len);
    //Result: recovered1="I love youð". Generally, it has form of "I love youX"
    //X can be any garbage chatacter, and each time I run the code, X is one different
    //character.
}

According to the accept answer, Solution is: updated my encrypt() like this:

char* encrypt(char * plainText, byte key[], int sizeKey, byte iv[], int sizeIV, long &len){
    string cipher = encryptString(plainText, key, sizeKey, iv, sizeIV);
    FileUtil::writeFile("ss1.txt", cipher, cipher.length());
    len = cipher.size() ;
     char * writable = new  char[len];
    std::copy(cipher.begin(), cipher.end(), writable);
    writable[len] = '\0'; // don't forget the terminating 0
    FileUtil::writeFile("w1.txt",writable, len);

    return writable;
}

Just allocate writeable's length = cipher's length. Set the terminator at writeble[len]

Answer 1

That tends to happen when you have things like buffer overruns and unterminated strings. If we look at your encrypt function we see a buffer overrun:

len = cipher.size() + 1;
char * writable = new  char[len];
std::copy(cipher.begin(), cipher.end(), writable);
writable[len] = '\0';

See here you allocated len bytes, where len is one larger than cipher. But when you terminate the string, you are using len to index which is out-of-bounds.

You should either use len-1 or cipher.size() for the terminator index.