Crypto | HexEncoder sometimes encodes incorrectly | HexEncoder not working consistently

I have written a simple encrypt & decrypt function and everything works. Except sometimes the HexEncoder seems to encode the cipher far too way small. Therefore the decryption fails. But this rarely happens. I have isolated the bug to the HexEncoder since the log Mismatch on encoding is only present when the problem appears. A bad workaround is to call the encrypt function again, which solves the problem. Not sure why, perhaps because of a newly generated IV but this seems unlikely to me.

Am I doing something wrong, undefined behaviour? Or is this a bug from the HexEncoder?

Encrypt function:

// Encrypt data.
template <typename Encryption> static inline
int     encrypt(Encryption& m_enc, str_t& encrypted, const str_t& data) {

    // Generate & set iv.
    CryptoPP::byte iv [CryptoPP::AES::BLOCKSIZE];
    rand.GenerateBlock(iv, CryptoPP::AES::BLOCKSIZE);
    m_enc.Resynchronize(iv, CryptoPP::AES::BLOCKSIZE);

    // Encrypt.
    str_t cipher;
    cipher.resize(data.len()   CryptoPP::AES::BLOCKSIZE);
    cipher.len() = data.len()   CryptoPP::AES::BLOCKSIZE;
    CryptoPP::ArraySink encrypt_sink(
        (Byte*) cipher.data(),
        cipher.len()
    );
    try {
        CryptoPP::StringSource(
            data.data(),
            true,
            new CryptoPP::StreamTransformationFilter(m_enc, new CryptoPP::Redirector(encrypt_sink))
        );
    } catch (...) {
        return crypto::error::encrypt;
    }
    //print("Cipher 1: ", cipher);

    // Set cipher text length now that its known
    if (cipher.len() != encrypt_sink.TotalPutLength()) {
        print("Mismatch on ciper; expected: ", cipher.len(), " sinked: ", encrypt_sink.TotalPutLength());
    }
    cipher.resize(encrypt_sink.TotalPutLength());
    cipher.len() = encrypt_sink.TotalPutLength();
    //print("Cipher 2: ", cipher);

    // Encode.
    encrypted.resize(cipher.len() * 2);
    encrypted.len() = cipher.len() * 2;
    CryptoPP::ArraySink encode_sink((Byte*) encrypted.data(), encrypted.len());
    try {
        CryptoPP::StringSource(
            cipher.data(),
            true,
            new CryptoPP::HexEncoder(
                new CryptoPP::Redirector(encode_sink)
            )
        );
    } catch (...) {
        encrypted.len() = 0;
        return crypto::error::encode;
    }
    print("Encoded cipher 1: ", encrypted);

    // Set encrypted length.
    if (encrypted.len() != encode_sink.TotalPutLength()) {
        print("BUG -> Mismatch on encoding; expected: ", encrypted.len(), " sinked: ", encode_sink.TotalPutLength());
        //str_t shortened;
        //shortened.resize(encode_sink.TotalPutLength());
        //shortened.concat_no_resize(encrypted.data(), encode_sink.TotalPutLength());
        //encrypted.swap(shortened);
        return encrypt(m_enc, encrypted, data);
    }
    encrypted.resize(encode_sink.TotalPutLength());
    encrypted.len() = encode_sink.TotalPutLength();
    print("Encoded cipher 2: ", encrypted);

    // Encode IV.
    str_t encoded_iv;
    encoded_iv.resize(CryptoPP::AES::BLOCKSIZE * 2);
    encoded_iv.len() = CryptoPP::AES::BLOCKSIZE * 2;
    CryptoPP::ArraySink iv_sink((Byte*) encoded_iv.data(), encoded_iv.len());
    try {
        CryptoPP::ArraySource(
            iv,
            CryptoPP::AES::BLOCKSIZE,
            true,
            new CryptoPP::HexEncoder(
                new CryptoPP::Redirector(iv_sink)
            )
        );
    } catch (...) {
        encrypted.len() = 0;
        return crypto::error::encode;
    }

    // Set encoded iv length.
    if (encoded_iv.len() != iv_sink.TotalPutLength()) {
        print("Mismatch on encoding iv; expected: ", encoded_iv.len(), " sinked: ", iv_sink.TotalPutLength());
    }
    encoded_iv.resize(iv_sink.TotalPutLength());
    encoded_iv.len() = iv_sink.TotalPutLength();
    print("Encoded IV: ", encoded_iv);

    // Concat IV.
    encoded_iv.concat(encrypted);
    encrypted.swap(encoded_iv);

    return 0;
}

Decrypt function:

// Decrypt data.
template <typename Decryption> static inline
int     decrypt(Decryption& m_dec, str_t& decrypted, const str_t& data) {

    // Decode.
    str_t decoded;
    decoded.resize(data.len() / 2);
    decoded.len() = data.len() / 2;

    try {
        CryptoPP::StringSource(
            data.data(),
            true,
            new CryptoPP::HexDecoder(
                new CryptoPP::ArraySink((Byte*) decoded.data(), decoded.len())
            )
        );
    } catch (...) {
        return crypto::error::decode;
    }
    // Skip for now.
    m_dec.Resynchronize((Byte*) decoded.data(), CryptoPP::AES::BLOCKSIZE);

    // Recovered text will be less than cipher text
    decrypted.resize(decoded.len() - CryptoPP::AES::BLOCKSIZE);
    decrypted.len() = decoded.len() - CryptoPP::AES::BLOCKSIZE;
    CryptoPP::ArraySink rs((Byte*) decrypted.data(), decrypted.len());

    try {
        CryptoPP::StringSource(
            (Byte*) decoded.data()   CryptoPP::AES::BLOCKSIZE,
            decoded.len(),
            true,
            new CryptoPP::StreamTransformationFilter(
                m_dec,
                new CryptoPP::Redirector(rs),
                CryptoPP::BlockPaddingSchemeDef::BlockPaddingScheme::NO_PADDING
            )
        );
    } catch (...) {
        decrypted.len() = 0;
        return crypto::error::decrypt;
    }

    // Set recovered text length now that its known
    decrypted.resize(rs.TotalPutLength());
    decrypted.len() = rs.TotalPutLength();

    return 0;
}

Testing:

...
str_t data = "Hello World!", encrypted, decrypted;
aes.encrypt(encrypted, data);
aes.decrypt(decrypted, encrypted);
print("Encrypted: ", encrypted);
print("Decrypted: ", decrypted);

This are the normal logs (without the bug occurence):

Mismatch on ciper; expected: 28 sinked: 16
Encoded cipher 1: 04A2C4FB074F6ACBA996239224BD5F77
Encoded cipher 2: 04A2C4FB074F6ACBA996239224BD5F77
Encoded IV: 02466AEBCF4AC2066CE2E144FC5B71C8
Encrypted: 02466AEBCF4AC2066CE2E144FC5B71C804A2C4FB074F6ACBA996239224BD5F77
Decrypted: Hello World!

This are the logs when the bug occurs (with the recursive encrypt call commented out):

Decoded: Hello World!
Mismatch on ciper; expected: 28 sinked: 16
Encoded cipher 1: C97BFE
BUG -> Mismatch on encoding; expected: 32 sinked: 6
Encoded cipher 2: C97BFE
Encoded IV: 06E05C3DAE3E9D6DAC8971E5C9CA0A1A
Encrypted: 06E05C3DAE3E9D6DAC8971E5C9CA0A1AC97BFE
Decrypted:

This are the logs when the bug occurs with the workaround:

Mismatch on ciper; expected: 28 sinked: 16
Encoded cipher 1: A0
BUG -> Mismatch on encoding; expected: 32 sinked: 2
Mismatch on ciper; expected: 28 sinked: 16
Encoded cipher 1: 883A8A644E5B50067A
BUG -> Mismatch on encoding; expected: 32 sinked: 18
Mismatch on ciper; expected: 28 sinked: 16
Encoded cipher 1: 58AD2010C761215422F89D42FC2F2396
Encoded cipher 2: 58AD2010C761215422F89D42FC2F2396
Encoded IV: BE361B7D8C9144052220E143AD54CB63
Encrypted: BE361B7D8C9144052220E143AD54CB6358AD2010C761215422F89D42FC2F2396
Decrypted: Hello World!

CodePudding user response：

I don't know what a str_t is, but since it has a .data() method I assume that returns the underlying char*. Since you are not specifying a length in the CryptoPP::StringSource, it assumes the input stops at the first NUL byte. For plaintext input this is often fine but cipher data MAY contain embedded NULs.

Instead, either pass the length of your cipher explicitly:

CryptoPP::StringSource(
            cipher.data(),
            cipher.len(),
            true,
            new CryptoPP::HexEncoder(
                new CryptoPP::Redirector(encode_sink)
            )
        );

Or, better, convert the str_t into a std::string that knows its own size and pass that as argument.