BLAKE2b_Update can pass NULL to memcpy (undefined behavior)

[guidovranken]

The following code will result in the second memcpy in BLAKE2b_Update having NULL as an argument:

C++:

#include <openssl/evp.h>
#include <vector>

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }
#define CF_CHECK_NE(expr, res) if ( (expr) == (res) ) { goto end; }

int main(void)
{
    std::vector<uint8_t> empty;

    EVP_MD_CTX* ctx = nullptr;
    const EVP_MD* md = nullptr;

    /* Initialize */
    {
        CF_CHECK_NE(md = EVP_blake2b512(), nullptr);
        CF_CHECK_NE(ctx = EVP_MD_CTX_create(), nullptr);
        CF_CHECK_EQ(EVP_DigestInit_ex(ctx, md, nullptr), 1);
    }

    /* Process */
    CF_CHECK_EQ(EVP_DigestUpdate(ctx, empty.data(), empty.size() ), 1);

    /* Finalize */
    {
        unsigned int len = -1;
        unsigned char md[EVP_MAX_MD_SIZE];
        CF_CHECK_EQ(EVP_DigestFinal_ex(ctx, md, &len), 1);
    }

end:
    EVP_MD_CTX_destroy(ctx);

    return 0;
}

I'm specifically using C++ and an std::vector as input here, because it demonstrates that EVP_DigestUpdate can reasonably receive a NULL pointer for its data parameter and 0 for its size parameter; an empty vector, as in this example, will (or can) return NULL from its data method.

To put it differently:

std::vector<uint8_t> empty;
uint8_t* p = empty.data();
/* p is now NULL */
size_t size = empty.size();
/* size is now 0 */

/* So this: */
EVP_DigestUpdate(ctx, empty.data(), empty.size());
/* equals: */
EVP_DigestUpdate(ctx, NULL, 0);

Passing NULL to memcpy is undefined behavior and can lead to crashes: https://gcc.gnu.org/gcc-4.9/porting_to.html
Hence it would be best if the second memcpy gets wrapped in something like if ( datalen ) { memcpy(c->buf + c->buflen, in, datalen); }

[kroeckx]

I think passing NULL to EVP_DigestUpdate is a misuse of the API, but if you really want, you can just let it return when the size is 0.

[guidovranken]

you can just let it return when the size is 0

In my opinion that would be better. This is the only instance of NULL as a parameter to memcpy across all digests and symmetric ciphers in OpenSSL I found with fuzz testing.
I can make a PR for the proposed change. But if you prefer to leave it as it is, that's ok, and feel free to close the issue.

[mattcaswell]

My proposed fix for this in #8587.