High severity vulnerability in phpseclib <3.0.36

[hoels]

There are 2 high severity vulnerabilities in the phpseclib in versions below 3.0.36 (see CVE-2024-27354 and CVE-2024-27355). Is there a reason for fixing a specific library version? I feel like this causes more harm than good and would recommend setting the version to at least "3.0.*", preferably even "^3.0", as there should be no breaking changes until 4.0.

[mrts]

Thanks for bringing this up! Yes, we noticed this and we will deliver fixed versions soon.

Regarding your question if there is a reason for fixing a specific library version - yes, we have taken a careful approach to dependencies until now, and with a reason, you may have noticed that CVE-2024-27354 was introduced when attempting to fix CVE-2023-27560. Our process requires us to test the dependencies before we introduce a new version.

Having said this, I will bring your suggestion up with the project leads, we may reconsider our approach.

[mrts]

The project leads decided that we will continue fixing a specific library versions and the careful upgrade approach as long as phpseclib makes changes in patch level versions that break our tests. See for example this change where bumping phpseclib from 3.0.19 to 3.0.34 required some changes in code, or this discussion.

We fully agree with you in principle and would like to do as you proposed, but phpseclib is not there yet unfortunately.

[hoels]

Hey @mrts, thanks for the fast response! I had a look at the changes you mentioned when bumping from 3.0.19 to 3.0.34 and compared the two tags in the phpseclib.

Regarding the first change in src/OcspResponse.php, there were no functional changes in the decodeBER method between the two versions. The method already returned either an array or null, so the change in the ocsp-php library looks more like a bugfix to me than a necessary change because of the new version.

Regarding the second change in src/certificate/CertificateLoader.php, the exception you are catching is thrown by the libsodium method sodium_base642bin that is used by phpseclib in phpseclib/Common/Functions/String.php in base64_decode. In my opinion, the method itself should either catch the exception or add it to the doc comment, but, as for the other change, this one was not required due to changes in the phpseclib. It may have been caused by changes in libsodium or has already been an issue before.

I didn't do an in-depth analysis, but to me it looks like pretty much all changes between 3.0.19 and 3.0.34 were changes to catch previously undefined behaviour or bugfixes, but no changes in logic that may break features. Out of curiosity, I downloaded the current version of ocsp-php, downgraded phpseclib to 3.0.19, reverted only the two changes made in the commit you referenced, and ran the tests. Two tests failed, regardless of whether 3.0.19 or 3.0.34 was used. This indicates that the test failures were not introduced because of changes in the phpseclib but because of other factors.

You probably have better insights into the phpseclib but I would still recommend to not fix a version for security and compatibility reasons and reconsider your decision with the project leads. Even though the fix of one vulnerability may lead to another, it is important to be able to react to new findings and security fixes as fast as possible. Having to wait for a new release of an additional library because of fixed versions slows down this process. Right now, unfortunately, relying on the ocsp-php library means accepting high severity vulnerabilities in your infrastructure, even though a fix has already been available for over a week. I agree that you should be careful with upgrades for production applications, but that's what the composer.lock takes care of. For libraries, this is the wrong approach in my opinion and also pretty uncommon in the open-source world.

[mrts]

Thanks for taking time to look into this more deeply! Fair points, I'll bring this up again and get back soon.

[mrts]

We will set the phpseclib version to "3.0.*" and release v1.1.0 soon. We will include composer.lock for reference so that integrators can know which version of phpseclib we have tested with.

[hoels]

I think that‘s a good solution. Thanks a lot for reconsidering this!

[mrts]

Here's our new policy for reference: https://github.com/web-eid/ocsp-php?tab=readme-ov-file#phpseclib-versioning-policy