Have legacy blake2 EVP structure use base blake2 implementation #22079
Conversation
For some reason, the code here was made to got through the provider specific init functions. This is very very dangerous if the provider specific functions were to change in any way (such as changes to the implementation context structure). Instead, use the init functions from the base blake2 implementations directly.
levitte
added
branch: master
t8m
added
triaged: refactor
paulidale
added
approval: done
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
|
Merged. |
For some reason, the code here was made to got through the provider specific init functions. This is very very dangerous if the provider specific functions were to change in any way (such as changes to the implementation context structure). Instead, use the init functions from the base blake2 implementations directly. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #22079)
|
Hmm.. there is a new Coverity issue as a consequence of this. Not sure why - maybe it just made it possible for Coverity to uncover this or it is a false positive, it needs to be investigated. |
| { | ||
| BLAKE2S_PARAM P; | ||
|
|
||
| ossl_blake2s_param_init(&P); |
There was a problem hiding this comment.
This introduces an uninitialised read in ossl_blake2s_param_init()
There was a problem hiding this comment.
False positive... ossl_blake2s_param_init() intializes that very structure, it's utter silliness to initialize it before that!
There was a problem hiding this comment.
Actually not a false positive....but a mistake by me putting this comment on the wrong line. It's actually the call to ossl_blake2b_param_init below that is the uninit read.
There was a problem hiding this comment.
Did you perhaps mean ossl_blake2b_init() (i.e. no param)?
There was a problem hiding this comment.
No? ossl_blake2b_param_init is called on line 29 in this file and reads the uninitialised P->digest_length value
There was a problem hiding this comment.
Oh, that hack. Ok. Well, I hope my fix is good enough
For some reason, the code here was made to got through the provider specific init functions. This is very very dangerous if the provider specific functions were to change in any way (such as changes to the implementation context structure). Instead, use the init functions from the base blake2 implementations directly. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl/openssl#22079) Signed-off-by: fly2x <fly2x@hitls.org>
For some reason, the code here was made to got through the provider
specific init functions. This is very very dangerous if the provider
specific functions were to change in any way (such as changes to the
implementation context structure).
Instead, use the init functions from the base blake2 implementations
directly.