Comment 16 for bug 1994165

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

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]:
"""
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().

1. https://launchpadlibrarian.net/692262295/buildlog_ubuntu-jammy-amd64.openssl_3.0.2-0ubuntu1.12_BUILDING.txt.gz
2. https://github.com/openssl/openssl/pull/18876#issuecomment-1323830916