Strange problem with decryption of AES using OpenSSL, Gets padded with same looking junk, Base64 padding issue

Question

Can someone tell me whats wrong with my code?
It works fine in my test example.. but when I use it in production model it decrypts the string but adds a padded symbol to maintain some kind of block size or something. I didn't post my encrypt/decrypt methods as they would make this post too big, plus they work fine as my test example decrypts and encrypts properly, ini.GetValue is a INI retrieval method there is nothing wrong with it, plus you can see the base64 size is the same as the example code, so I believe it works fine, I never had any problems with it before without encryption when I used it, it returns a const char*

The problem is known as you can see the production code ciphertext has appended to it 2 null bytes which I find strange becasue both codes are pretty much identical, I'm not good at C++ so I'm probably overlooking some basic char array stuff

The encryption code I use is AES-256-CBC from OpenSSL 1.1.1

Look at my outputs to see whats wrong.

Good looking example code:

Ciphertext is:
000000: 7a e1 69 61 65 bb 74 ad 1a 68 8a ae 73 70 b6 0e  z.iae.t..h..sp..
000010: 4f c9 45 9b 44 ca e2 be e2 aa 16 14 cd b1 79 7b  O.E.D.........y{
000020: 86 a5 92 26 e6 08 3e 55 61 4e 60 03 50 f3 e4 c1  ...&..>UaN`.P...
000030: fe 5a 2c 0b df c9 1b d8 92 1f 48 75 0d f8 c2 44  .Z,.......Hu...D
Base64 (size=88):
000000: 65 75 46 70 59 57 57 37 64 4b 30 61 61 49 71 75  euFpYWW7dK0aaIqu
000010: 63 33 43 32 44 6b 2f 4a 52 5a 74 45 79 75 4b 2b  c3C2Dk/JRZtEyuK+
000020: 34 71 6f 57 46 4d 32 78 65 58 75 47 70 5a 49 6d  4qoWFM2xeXuGpZIm
000030: 35 67 67 2b 56 57 46 4f 59 41 4e 51 38 2b 54 42  5gg+VWFOYANQ8+TB
000040: 2f 6c 6f 73 43 39 2f 4a 47 39 69 53 48 30 68 31  /losC9/JG9iSH0h1
000050: 44 66 6a 43 52 41 3d 3d                          DfjCRA==
b cip len = 64
a cip len = 16
plain b = 0
plain a = 3
Decrypted text is:
wtf
Decrypted base64 is:
wtf
000000: 77 74 66 00                                      wtf.

Bad production code example:

Base64 (size=88)
000000: 6a 7a 48 30 46 71 73 54 45 47 4d 76 2f 67 76 59  jzH0FqsTEGMv/gvY
000010: 4d 73 34 54 2f 39 58 32 6c 37 54 31 4d 6d 56 61  Ms4T/9X2l7T1MmVa
000020: 36 45 4f 38 52 64 45 57 42 6b 65 48 71 31 31 45  6EO8RdEWBkeHq11E
000030: 39 2b 77 37 47 4e 49 4a 47 4a 71 42 55 74 54 70  9+w7GNIJGJqBUtTp
000040: 30 36 58 46 31 4d 66 45 79 44 45 71 5a 69 58 54  06XF1MfEyDEqZiXT
000050: 79 45 53 6b 65 41 3d 3d                          yESkeA==
Ciphertext is:
000000: 8f 31 f4 16 ab 13 10 63 2f fe 0b d8 32 ce 13 ff  .1.....c/...2...
000010: d5 f6 97 b4 f5 32 65 5a e8 43 bc 45 d1 16 06 47  .....2eZ.C.E...G
000020: 87 ab 5d 44 f7 ec 3b 18 d2 09 18 9a 81 52 d4 e9  ..]D..;......R..
000030: d3 a5 c5 d4 c7 c4 c8 31 2a 66 25 d3 c8 44 a4 78  .......1*f%..D.x
000040: 00 00                                            ..
b cip len = 65
a cip len = 17
crypt miss-match
plain b = 16
crypt write fail
plain a = 16
000000: 77 74 66 09 09 09 09 09 09 09 09 05 05 05 05 05  wtf.............

Here are my codes as you can see they both look very similar so I don't understand whats the problem.

Here is a little helper function for hexdump outputs I use.

void Hexdump(void* ptr, int buflen)
{
    unsigned char* buf = (unsigned char*)ptr;
    int i, j;
    for (i = 0; i < buflen; i += 16) {
        myprintf("%06x: ", i);
        for (j = 0; j < 16; j++)
            if (i + j < buflen)
                myprintf("%02x ", buf[i + j]);
            else
                myprintf("   ");
        myprintf(" ");
        for (j = 0; j < 16; j++)
            if (i + j < buflen)
                myprintf("%c", isprint(buf[i + j]) ? buf[i + j] : '.');
        myprintf("\n");
    }
}

char* base64(const unsigned char* input, int length) {
    const auto pl = 4 * ((length + 2) / 3);
    auto output = reinterpret_cast<char*>(calloc(pl + 1, 1)); //+1 for the terminating null that EVP_EncodeBlock adds on
    const auto ol = EVP_EncodeBlock(reinterpret_cast<unsigned char*>(output), input, length);
    if (pl != ol) { myprintf("b64 calc %d,%d\n",pl, ol); }
    return output;
}

unsigned char* decode64(const char* input, int length) {
    const auto pl = 3 * length / 4;
    auto output = reinterpret_cast<unsigned char*>(calloc(pl + 1, 1));
    const auto ol = EVP_DecodeBlock(output, reinterpret_cast<const unsigned char*>(input), length);
    if (pl != ol) { myprintf("d64 calc %d,%d\n", pl, ol); }
    return output;
}

Here is the test example that works fine.

    /* enc test */
    /* Message to be encrypted */
    unsigned char* plaintext = (unsigned char*)"wtf";

    /*
     * Buffer for ciphertext. Ensure the buffer is long enough for the
     * ciphertext which may be longer than the plaintext, depending on the
     * algorithm and mode.
     */
    unsigned char* ciphertext = new unsigned char[128];

    /* Buffer for the decrypted text */
    unsigned char decryptedtext[128];

    int decryptedtext_len, ciphertext_len;

    /* Encrypt the plaintext */
    ciphertext_len = encrypt(plaintext, strlen((char*)plaintext), ciphertext);

    /* Do something useful with the ciphertext here */
    myprintf("Ciphertext is:\n");
    Hexdump((void*)ciphertext, ciphertext_len);
    myprintf("Base64 (size=%d):\n", strlen(base64(ciphertext, ciphertext_len)));
    Hexdump((void*)base64(ciphertext, ciphertext_len), 4 * ((ciphertext_len + 2) / 3));

    /* Decrypt the ciphertext */
    decryptedtext_len = decrypt(ciphertext, ciphertext_len, decryptedtext);

    /* Add a NULL terminator. We are expecting printable text */
    decryptedtext[decryptedtext_len] = '\0';

    /* Show the decrypted text */
    myprintf("Decrypted text is:\n");
    myprintf("%s\n", decryptedtext);
    myprintf("Decrypted base64 is:\n");
    myprintf("%s\n", decode64(base64(decryptedtext, decryptedtext_len), 4 * ((decryptedtext_len + 2) / 3)));
    Hexdump(decode64(base64(decryptedtext, decryptedtext_len), 4 * ((decryptedtext_len + 2) / 3)), 4 * ((decryptedtext_len + 2) / 3));
    /* enc test end */

Here is the bad production code:

    //Decrypt the username
    const char* b64buffer = ini.GetValue("Credentials", "SavedPassword", "");
    int b64buffer_length = strlen(b64buffer);
    myprintf("Base64 (size=%d)\n", b64buffer_length);
    Hexdump((void*)b64buffer, b64buffer_length);
    int decryptedtext_len;
    int decoded_size = 3 * b64buffer_length / 4;
    unsigned char* decryptedtext = new unsigned char[decoded_size];
    //unsigned char* ciphertext = decode64(b64buffer, b64buffer_length); //had this before same problem as below line, this worked without initializing new memory I perfer to fix this back up
    unsigned char* ciphertext = new unsigned char[decoded_size];
    memcpy(ciphertext, decode64(b64buffer, b64buffer_length), decoded_size); //same problem as top line.
    myprintf("Ciphertext is:\n");
    Hexdump((void*)ciphertext, decoded_size);

    /* Decrypt the ciphertext */
    decryptedtext_len = decrypt(ciphertext, decoded_size - 1, decryptedtext);
    /* Add a NULL terminator. We are expecting printable text */
    decryptedtext[decryptedtext_len] = '\0';
    Hexdump(decryptedtext, decryptedtext_len);
    strcpy(password_setting, (char*)decryptedtext); //save decrypted password back

    delete[] decryptedtext;
    delete[] ciphertext;

Answer 1

In the example that works, you get ciphertext_len directly from the encryption function. When you display the ciphertext, you use this length.

In the "bad production code", you calculate decoded_size from the length of the Base64 data. However, Base64 encoded data always has a length that is a multiple of 4. If the original data size is not a multiple of 3, then there are one or two padding characters added to the string. In both of your examples, you have two of these characters, the '=' at the end of the Base64 data.

When calculating the length of the decrypted data, you need to account for these bytes. If there are no '=' characters at the end of the string, use the length that you calculated (3 * N / 4). If there is one '=' character, reduce that calculated length by 1, and if there are two '=' characters, reduce the calculated length by 2. (There will not be 3 padding characters.)

Edit: Here is my fix: (sspoke)

char* base64(const unsigned char* input, int length) {
    const auto pl = 4 * ((length + 2) / 3);
    auto output = reinterpret_cast<char*>(calloc(pl + 1, 1)); //+1 for the terminating null that EVP_EncodeBlock adds on
    const auto ol = EVP_EncodeBlock(reinterpret_cast<unsigned char*>(output), input, length);
    if (pl != ol) { printf("encode64 fail size size %d,%d\n",pl, ol); }
    return output;
}

unsigned char* decode64(const char* input, int* length) {
    //Old code generated base length sizes because it didn't take into account the '==' signs.
    const auto pl = 3 * *length / 4;
    auto output = reinterpret_cast<unsigned char*>(calloc(pl + 1, 1));
    const auto ol = EVP_DecodeBlock(output, reinterpret_cast<const unsigned char*>(input), *length);
    if (pl != ol) { printf("decode64 fail size size %d,%d\n", pl, ol); }
    
    //Little bug fix I added to fix incorrect length's because '==' signs are not considered in the output. -sspoke
    if (*length > 3 && input[*length - 1] == '=' && input[*length - 2] == '=')
        *length = ol - 2;
    else if (*length > 2 && input[*length - 1] == '=')
        *length = ol - 1;
    else
        *length = ol;

    return output;
}