a) The original bug: CMS_final() was not taking into account the result of CMS_dataFinal() when returning its return code. It might be that CMS_dataFinal() failed, in which case an error would even be raised, but the return code of CMS_final() would be that of SMIME_crlf_copy().
b) While fixing (a), it was noticed that tons of other places in the code were not checking the result code of SMIME_crlf_copy(). It looks like this function returns a failure (0) when BIO_new() fails, so, a memory allocation.
I checked an openssl build log[1], and there are tests for CMS, and none of them failed.
The failure case is explained in [2]:
"""
It means some broken message that will later fail parsing/decryption/signature verification can be produced.
"""
So, my ponderings:
- for (a), was there perhaps a test case added to the openssl case to cover for that mistake? If not, could we come up with one in the test suite?
- the fix for (b) was a drive-by fix, and seems correct, but it is touching more code. What do you think about isolating the fix in this SRU to just the CMS_final() function? Pros and cons? So we fix the CMS_final() function, in the sense that we will keep checking both the return code of SMIME_crlf_copy() and CMS_dataFinal().
There are two changes here:
a) The original bug: CMS_final() was not taking into account the result of CMS_dataFinal() when returning its return code. It might be that CMS_dataFinal() failed, in which case an error would even be raised, but the return code of CMS_final() would be that of SMIME_crlf_copy().
b) While fixing (a), it was noticed that tons of other places in the code were not checking the result code of SMIME_crlf_copy(). It looks like this function returns a failure (0) when BIO_new() fails, so, a memory allocation.
I checked an openssl build log[1], and there are tests for CMS, and none of them failed.
The failure case is explained in [2]: decryption/ signature verification can be produced.
"""
It means some broken message that will later fail parsing/
"""
So, my ponderings:
- for (a), was there perhaps a test case added to the openssl case to cover for that mistake? If not, could we come up with one in the test suite?
- the fix for (b) was a drive-by fix, and seems correct, but it is touching more code. What do you think about isolating the fix in this SRU to just the CMS_final() function? Pros and cons? So we fix the CMS_final() function, in the sense that we will keep checking both the return code of SMIME_crlf_copy() and CMS_dataFinal().
1. https:/ /launchpadlibra rian.net/ 692262295/ buildlog_ ubuntu- jammy-amd64. openssl_ 3.0.2-0ubuntu1. 12_BUILDING. txt.gz /github. com/openssl/ openssl/ pull/18876# issuecomment- 1323830916
2. https:/