check the return value of EVP_MD_CTX_create(EVP_MD_CTX_new) #8133
Conversation
|
Shouldn't the init function rather return an error to then have |
|
Yes, it is best for this case. However, I hesitated to do that yesterday as there are other implementations under different |
|
Sorry. I made mistakes that commit to this branch. later I will rollback it. |
| @@ -288,6 +288,10 @@ CURLcode Curl_output_aws_sigv4(struct Curl_easy *data, bool proxy) | |||
| post_data_len = (size_t)data->set.postfieldsize; | |||
| Curl_sha256it(sha_hash, | |||
| (const unsigned char *) post_data, post_data_len); | |||
| if(!(char *) sha_hash) { | |||
| goto fail; | |||
| } | |||
There was a problem hiding this comment.
please make Curl_sha256it return error/success instead and check that instead of relying on the output data. Yes, that's a bigger change but is much cleaner and nicer code.
|
Thanks! That was an oversight in 94696e1. I agree with @bagder , the error should be handled by the caller. That would mean changing the signature of
The other implementations could just always return |
|
I changed the return type of |
| my_sha256_update(&ctx, input, curlx_uztoui(length)); | ||
| my_sha256_final(output, &ctx); | ||
| return ret; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
I can't comment on line 527, but the HMAC_hinit_func signature (and the MD5 init function) would have to be updated too to return CURLcode. Unfortunately the CURLX_FUNCTION_CAST meant for the parameter hides that. Looks good to me otherwise!
There was a problem hiding this comment.
Sorry, I forgot to update it though you reminded me. I will do it after work...
There was a problem hiding this comment.
Thanks, looks good to me now! Just waiting if @bagder wants to have another look too.
|
Thanks! |
check the return value of EVP_MD_CTX_create(i.e. EVP_MD_CTX_new) in lib/sha256.c:87.
If some memory allocation errors occur, EVP_ MD_ CTX_ New() will return null, then EVP_DigestInit_ex(), EVP_DigestUpdate() and EVP_DigestFinal_ex() will make wrong memory access. So it is better to check the return value of it.