CMS: resolve the libctx if passed NULL#13657
Conversation
d2i_CMS_bio(), d2i_CMS_ContentInfo() and SMIME_read_CMS_ex() would behave differently if passed a cms == NULL or if *cms == NULL although the API is such that both calls are effectively equivalent. Drop an incorrect NULL check and resolve the libcontexts in both cases. This could lead to a NULL pointer dereference in CMS_SignerInfo_verify() via CMS_verify(). fixes: openssl#13624 CLA: trivial
t8m
added
approval: review pending
|
BTW, I agree with CLA: trivial. |
slontis
left a comment
slontis
left a comment
There was a problem hiding this comment.
The point here is that if you want to use libctx and propq they need to be setup before this call..
i.e:
ret = CMS_ContentInfo_new_ex(libctx, propq);
and then pass this to the d2i.. otherwise the resolve does not really do much..
If we have a NULL access somewhere then that needs to be addressed instead of doing this.
|
@slontis Thanks. I gave you a reproducer for the NULL access in the
linked issue. Note that when doing `cms = d2i_CMS_bio(mft_bio, NULL);`
the program crashes in `CMS_verify()`. When doing
`cms = NULL; d2i_CMS_bio(mft_bio, &cms);`, it does not and the manifest verifies successfully.
Both work the same on OpenSSL 1.1.1, i.e., they allocate the CMS_ContentInfo for me.
|
|
@slontis It may well be that this masks another bug. From what you are saying, I still think these checks are incorrect. If you actually want to require that libctx and propq be set up before resolving, the checks should not be All I'm saying is that the two cases I'll happily rebase, but I would appreciate input on how to fix the missing libctx issue in the crash I'm hitting. |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago |
|
I'm closing this as the two commits in PR #13668 (which has just been merged) contain these fixes next to a lot of other stuff. |
4 participants
d2i_CMS_bio(), d2i_CMS_ContentInfo() and SMIME_read_CMS_ex() would
behave differently if passed a cms == NULL or if *cms == NULL although
the API is such that both calls are effectively equivalent. Drop an
incorrect NULL check and resolve the libcontexts in both cases.
This could lead to a NULL pointer dereference in CMS_SignerInfo_verify()
via CMS_verify().
Fixes: #13624
CLA: trivial