problem on srtp_aes_icm_openssl_encrypt

[juancarlossanchez]

EVP_EncryptFinal_ex is writing in the same position as EVP_EncryptUpdate, this means that the encrypted data could be overwritten:

    if (!EVP_EncryptFinal_ex(c->ctx, buf, &len)) {
        return srtp_err_status_cipher_fail;
    }

should be:

    if (!EVP_EncryptFinal_ex(c->ctx, buf + len, &len)) {
        return srtp_err_status_cipher_fail;
    }

[pabuhler]

Based on documentation I belive you to be correct but in practice I guess this never happens. Still it would be best to make it correct, will you make a PR? Also I think you need to use enc_len at this point as it will contain the length of data written to buf previously. Something like:

enc_len = len; len = 0; if (!EVP_EncryptFinal_ex(c->ctx, buf + enc_len , &len)) {

[juancarlossanchez]

yes, I go to create the pull request

[JonathanLennox]

Yes, EncryptFinal_ex doesn't actually write anything for ICM (it's a stream cipher, so there's no trailer to it), but it's good to fix this in case this code is ever copied for any other cipher modes.

[davidben]

@pabuhler That works too, but it's not necessarily since there's only one call to EVP_EncryptUpdate and &len is not an in/out param, just an out param. The original fix is sufficient.

[juancarlossanchez]

@davidben @pabuhler yes, I have created a pull request with the first fix.