On 10/24/2012 11:34 PM, Sherif Ramadan wrote:
On Thu, Oct 25, 2012 at 1:03 AM, JJ <ja...@php.net> wrote:
Hey all - I'd like start a discussion around pull request 221
(https://github.com/php/php-src/pull/221).
In short, there's a high volume of [incorrect] code out there which looks like:
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, true);
Instead of what, in all likelyhood, the code meant to do:
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, 2);
This is due to the convert_to_long_ex call which converts "true" to
1L. CURLOPT_SSL_VERIFYHOST being set to 1L bypasses common name
validation within libcurl.
My solution was to check the type for CURLOPT_SSL_VERIFYHOST: if it is
boolean and true, the opt value for libcurl is set to 2L.
I understand that engineers should have the proper option value to
begin with but weighing the impact of this (MITM attacks) against
doing what they probably meant anyways is worth the presumption.
Please discuss and adjust the patch if necessary.
- JJ
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
While I think it's a good idea to set the value of the option to 2, as
is recommended for production in the documentation, I think the idea
of implicitly converting a bool(true) to 2L internally might lead to
unexpected behavior since some people might actually depend on normal
PHP behavior to cast a bool(true) to 1 (and that might be what they
actually intended).
I understand there are people out there that don't read the
documentation and aren't aware of the difference between
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, 2); and curl_setopt($ch,
CURLOPT_SSL_VERIFYHOST, true); but still... I don't think this is a
good idea either.
We should probably just elaborate on this point a bit more in the
documentation. Perhaps add a note and an example to illustrate. I
notice that people tend to pay more attention to examples than
anything else in the docs.
Booleans ought to be 1 and 0. Casting a boolean to 2 is just wrong, a
way to fix badly written code a few people have written and in so doing
risk the breakage of far more code that is correct.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php