qa-base.php
59.4 KB
-
Do verify SSL/TLS certificates when using cURL · 8c6fd032
This is a security vulnerability fix. I've mailed it to you, but was told to just make a public PR, so I'll do. In line https://github.com/q2a/question2answer/blob/1af307ebab9c46fb61a4ec7a0640030bf132d8b4/qa-include/qa-base.php#L1884 you set CURLOPT_SSL_VERIFYPEER to false. According to the official CURL doc (https://curl.haxx.se/libcurl/c/CURLOPT_SSL_VERIFYPEER.html) that means it disables verifying of the peer SSL/TLS certificate completly. As the warning there also explains this allows man-in-the-middle attacks. Thus, any request to a HTTPS URL that usually is secured by HTTPS is basically broken by default in Q2A. IMHO this should never be the case. Even if some people may e.g. use broken cooperate proxies or so, this should still be no reason to disable it by default for anyone. It should at most be an optional setting (opt-in) and mentioned/explained somewhere (in the documentation) what it does. Implications: As I'm not exactly sure where this is all used, I can only guess. But as I know it is used for update checking the plugins, an attacker may e.g. fake the result and always respond with the current version, so one could prevent the admin from knowing there is a new version of a plugin available. And if a plugin e.g. fixed a security vulnerability, this may then prevent admins from getting to know of the update. Even if there were no immediate security risk caused by this, it is just not required to disable such a powerful security feature. So if it is done, it really needs a good reasoning/explanation as it is potentially a big security issue. Solution: My proposal would be just to remove this line in order to enable certificate verification again. (Even if an admin would want to disable it, they could still do it.) And if it is an issue with an outdated CaCert.pem bundle, admins should just be enoucuraged to update it, which can be done manually even. See https://stackoverflow.com/questions/2694787/how-can-i-set-curlopt-cainfo-globally-for-php-on-windows and the first comment on this PHP doc (https://www.php.net/manual/en/function.curl-setopt.php#110457). Also BTW, the current behaviour is inconsistent with the fallback to `file_get_contents`, which always verifies certificates.
rklec authored
×