Beginner's C++ memory allocation mess

Question

I am trying the code bellow. The thing is: some times work and some times doesn't! I am using the OpenSSL library, but I believe my mistake is something simple, C related.

int test_c(string to_sign, string key)
{
    const void* key_void = (const void*)key.c_str();
    int key_size = key.size();

    const unsigned char* data_char = (const unsigned char*)to_sign.c_str();
    int data_size = to_sign.size();

    unsigned int res_f_len = data_size*8;
    unsigned char* res_f = (unsigned char*) malloc(sizeof(unsigned char)*res_f_len);

    HMAC_CTX ctx;
    HMAC_CTX_init(&ctx);
    HMAC_Init_ex(&ctx, key_void, key_size, EVP_sha256(), NULL);
    HMAC_Update(&ctx, data_char, data_size);
    HMAC_Final(&ctx, res_f, &res_f_len);
    HMAC_CTX_cleanup(&ctx);

    return strlen(res_f);
}

When I say it doesn't work, I mean, for the same data and key, different results are returned!

If this is C, then you don't need casts around `void *`s (including the return value of `malloc()`). If this is C++, please don't tag with `c`. — , Dec 19 '13 at 00:29
Your use of strlen(res_f) indicates that the result string size is unknown. Do the HMAC functions fill/zero res_f? If not, malloc returns you a pointer to memory filled with undefined values (as in, garbage) and strlen works by looking for 0s. — Warty, Dec 19 '13 at 00:45
Clever guess. The result size is unknown, I know the size of the key on the other hand (60 bytes). HMAC_Final fills the res_f with data. — Blazer, Dec 19 '13 at 00:57
@Blazer: Yes, but does it fill it completely, or just enough to contain encoded data? You hand in the address to res_f_len to HMAC_Final. I went to github, searched "HMAC_Final", included C++ results only, and saw code similar to `uint32_t len = 0; HMAC_Final(&m_ctx, m_digest, &len); return std::vector(m_digest, m_digest + len);`. This hints to me that HMAC_Final mutates the length parameter and that you might want to use that. — Warty, Dec 19 '13 at 01:22
Why isn't the result length in `res_f_len`? It seems like a hashed result could have 0s in it, which would cause `strlen()` to return too small a number. It _also_ seems like the result could be entirely non-zero, in which case `strlen()` would stop on the first zero it finds in random garbage after the result. Only in the case where there's exactly 0 zero in the buffer, and it's the first byte after the result would `strlen()` return the answer you want, and that seems like a dicey proposition. — Joe Z, Dec 19 '13 at 02:33

score 1 · Accepted Answer · edited May 23 '17 at 12:28

Rather than return strlen(res_f); as the length of the output buffer, you need to return res_f_len.

There are three things that could happen with strlen() in your code:

If the HMAC message digest has a byte that equals zero, strlen will return a length that's shorter than the HMAC message digest.
If the HMAC message digest has no zero bytes, then strlen will read beyond the message digest, leading to one of two outcomes:
- strlen reads through garbage in memory after the message digest, eventually stopping on a random 0. strlen returns a value that's way too large.
- If you're exceedingly lucky, the first byte after the message digest is 0, in which case strlen returns the actual length of the message digest.

So, only in the last scenario will you get the right value. That pretty much seems wrong.

It appears tht HMAC_Final does update the length argument you pass to it. (See the example in the top-voted answer to this StackOverflow question.) That's why you pass a pointer-to-int. You should just return this updated value.

Owh, after wasting about 10 hours on this and messed up my whole program it was really exactly what you write here!!! Thanks a ton! — Blazer, Dec 19 '13 at 13:26

Beginner's C++ memory allocation mess

1 Answers1