Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] [off] PHP: a fractal of bad design

Posted by Adir Kuhn 
Ferenc Kovacs
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 11:00AM
On Mon, May 7, 2012 at 10:31 AM, Kris Craig <[email protected]> wrote:

> On Mon, May 7, 2012 at 12:28 AM, Arvids Godjuks <[email protected]
> >wrote:
>
> > Hello internals,
> >
> > I should voice my opinion that such things like comparing two strings
> > starting with numbers and that they resolve to actual integer/float for
> > comparation is bad, really bad. That just defies the logic and yealds
> > absolutly unexpected results. I pride myself that i know the juggling
> rules
> > well, but I'm shocked by this to say the least...
> > In my opinion this should change no matter the BC breaks it will create,
> > this one affects security big time. It's good I actually hash my
> passwords
> > in the MySQL and not on the PHP side, but I have seen hash comparations
> > with == all the time. And now that this has been discussed in detail I
> > expect this to be used as an attack method grow wide.
> > 07.05.2012 5:32 пользователь "Tjerk Anne Meesters" <[email protected]>
> > написал:
>
>
> Forgive me if I'm missing something, but why are people using == for
> security-sensitive string comparisons (like hashed passwords) in the first
> place?! If you needs something that's safe, isn't that what strcmp() and
> strcasecmp() are for? For my part, I pretty much never use == on string
> comparison, though admittedly that's probably just a matter of having
> having come from a C background before PHP.
>
> That being said, I agree that this *definitely* should be fixed if the
> examples cited are indeed accurate (I've been working with PHP for over 10
> years and I was never aware of this bizarre behavior, either). I don't
> know the history of this, but I at least would consider it to be a bug. A
> rather large one, in fact; though I think some of the fears expressed are a
> bit hyperbolic. And if you're fixing a serious bug or security
> vulnerability, as a general rule of thumb, this automatically supercedes
> any concerns regarding BC breakage IMHO. But if that really is a lingering
> concern, I'd suggest targetting the fix for PHP 6, since people would (or
> at least should) expect that some PHP 5 code may behave differently in PHP
> 6 anyway given that it's a major release
>
>
I agree with Rasmus (and others) on this one (
http://www.mail-archive.com/[email protected]/msg57949.html), type
numeric conversion on strings on both sides should only happen if both
strings can be represented as numbers without losing data/precision.
if one or both of the two sides can't be converted to numeric without data
loss, then we should compare them as strings.
the patch from Cataphract (attached at https://bugs.php.net/bug.php?id=54547)
implements this behavior

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Kris Craig
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 11:30AM
On Mon, May 7, 2012 at 1:46 AM, Gustavo Lopes <[email protected]>wrote:

> On Mon, 07 May 2012 10:31:00 +0200, Kris Craig <[email protected]>
> wrote:
>
> That being said, I agree that this *definitely* should be fixed if the
>>
>> examples cited are indeed accurate (I've been working with PHP for over
>> 10 years and I was never aware of this bizarre behavior, either). I don't
>> know the history of this, but I at least would consider it to be a bug.
>> A rather large one, in fact; though I think some of the fears expressed
>> are a bit hyperbolic. And if you're fixing a serious bug or security
>> vulnerability, as a general rule of thumb, this automatically supercedes
>> any concerns regarding BC breakage IMHO.
>>
>
> This has already been discussed in its own thread, see
> http://comments.gmane.org/**gmane.comp.php.devel/72790http://comments.gmane.org/gmane.comp.php.devel/72790; see also
> https://bugs.php.net/bug.php?**id=54547https://bugs.php.net/bug.php?id=54547
>
> Doing away entirely with this behavior (e.g. making " 2" == "2" compare
> false) would be a rather large BC break, that discussion is restricted to
> whether integers in strings should be treated differently from integer
> literals for comparison purposes when their range is exceeded. I wrote a
> patch for that, and, while not really caring one way or the other, I'm not
> convinced it's necessary and I some have consistency concerns (though float
> overflows already get a similar treatment).
>
> --
> Gustavo Lopes
>

Agreed. While it's no secret that I believe optional strong typing would
be helpful in this context, setting that aside I would definitely not want
to see implicit conversions like "2" == 2 or even " 2" == 2 go away (though
I do wish more PHP devs were familiar with the various trim() functions
lol).

That being said, this is what had me scratching my head from a previous
message in this thread:

>$a = "123ABF453..."; //a password
>$b = "123DFEABC..."; //another one
>if ($a == $b){
> //you're in.
>}


The thought that $a == $b would actually evaluate to TRUE is utterly
absurd.... Only catch is, it doesn't. As I was sharpening my pitchfork
getting ready to argue why this should be considered a bug and not
conflated with implicit conversions like " 2" == 2, I decided to do a
little test script against 5.3.11. I was unable to reproduce the behavior
being reported in the above example. In my tests, $a == $b returned FALSE.
As it should.

@Richard Could you clarify what you were trying to illustrate with that
code? I wasn't able to find anything wrong with how PHP handled it. So
either you were mistaken or I misunderstood your point (if the latter,
please tell me how so I can try again to repro the problem you're raising
concern about).

So with that in mind, the point I raised in the second paragraph of my last
email is moot because I was referring to behavior that doesn't actually
exist (again, unless I was testing for the wrong thing somehow?). Implicit
conversions like " 1" == 1.0 I fully support so long as they can both be
converted to numbers without loss of precision.

--Kris
Ferenc Kovacs
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 11:40AM
On Mon, May 7, 2012 at 11:22 AM, Kris Craig <[email protected]> wrote:

> On Mon, May 7, 2012 at 1:46 AM, Gustavo Lopes <[email protected]
> >wrote:
>
> > On Mon, 07 May 2012 10:31:00 +0200, Kris Craig <[email protected]>
> > wrote:
> >
> > That being said, I agree that this *definitely* should be fixed if the
> >>
> >> examples cited are indeed accurate (I've been working with PHP for over
> >> 10 years and I was never aware of this bizarre behavior, either). I
> don't
> >> know the history of this, but I at least would consider it to be a bug..
> >> A rather large one, in fact; though I think some of the fears expressed
> >> are a bit hyperbolic. And if you're fixing a serious bug or security
> >> vulnerability, as a general rule of thumb, this automatically supercedes
> >> any concerns regarding BC breakage IMHO.
> >>
> >
> > This has already been discussed in its own thread, see
> > http://comments.gmane.org/**gmane.comp.php.devel/72790<;
> http://comments.gmane.org/gmane.comp.php.devel/72790>;; see also
> > https://bugs.php.net/bug.php?**id=54547<;
> https://bugs.php.net/bug.php?id=54547>;
> >
> > Doing away entirely with this behavior (e.g. making " 2" == "2" compare
> > false) would be a rather large BC break, that discussion is restricted to
> > whether integers in strings should be treated differently from integer
> > literals for comparison purposes when their range is exceeded. I wrote a
> > patch for that, and, while not really caring one way or the other, I'm
> not
> > convinced it's necessary and I some have consistency concerns (though
> float
> > overflows already get a similar treatment).
> >
> > --
> > Gustavo Lopes
> >
>
> Agreed. While it's no secret that I believe optional strong typing would
> be helpful in this context, setting that aside I would definitely not want
> to see implicit conversions like "2" == 2 or even " 2" == 2 go away (though
> I do wish more PHP devs were familiar with the various trim() functions
> lol).
>
> That being said, this is what had me scratching my head from a previous
> message in this thread:
>
> >$a = "123ABF453..."; //a password
> >$b = "123DFEABC..."; //another one
> >if ($a == $b){
> > //you're in.
> >}
>
>
> The thought that $a == $b would actually evaluate to TRUE is utterly
> absurd.... Only catch is, it doesn't. As I was sharpening my pitchfork
> getting ready to argue why this should be considered a bug and not
> conflated with implicit conversions like " 2" == 2, I decided to do a
> little test script against 5.3.11. I was unable to reproduce the behavior
> being reported in the above example. In my tests, $a == $b returned FALSE.
> As it should.
>
>
juggling won't happen here, because both sides are strings, and they aren't
numerical strings, so that example was bogus indeed.
the discussion here is about whether it is not expected that the numerical
strings will compared as numbers even in they can only represented as
number if some kind of precision loss occurs.
about numerical strings:
http://php.net/manual/en/language.operators.comparison.php
"If you compare a number with a string or the comparison involves numerical
strings, then each string is converted to a
number<http://www.php.net/manual/en/language.types.string.php#language.types.string.conversion>;
and
the comparison performed numerically. These rules also apply to the
switchhttp://www.php.net/manual/en/control-structures.switch.php
statement.
The type conversion does not take place when the comparison is === or !==
as this involves comparing the type as well as the value."


--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Tjerk Anne Meesters
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 11:40AM
>
> I agree with Rasmus (and others) on this one
> (http://www.mail-archive.com/[email protected]/msg57949.html), type
> numeric conversion on strings on both sides should only happen if both
> strings can be represented as numbers without losing data/precision.
> if one or both of the two sides can't be converted to numeric without data
> loss, then we should compare them as strings.
> the patch from Cataphract (attached at
> https://bugs.php.net/bug.php?id=54547) implements this behavior
>

+1 ... that patch has been lying on a dusty shelf for more than a year =p

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Arvids Godjuks
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 12:50PM
Easy - you see == everywhere and === is used rarely, in docs you see it in
some places like strpos(). This is one thing that has to be communicated
through every channel available (including docs) with clear examples that
show why it should be used instead of ==.
Take me for example, I never had any idea that comparing two hashes that
start with numbers can generate a logical truth despite the hashes being
different. I haven't seen anything related to this in docs and never
stumbled upon in the internet, ever. So I use == for comparing strings all
the time. Now I will change that because I have the knowledge why I should
use === instead of ==.
Well, now I do know, from this mailing list (witch is for PHP development)
- not many user-land developers read this list.

And in my previous message I said essentially the same as you did, just in
different words and style. English is not my native language and I have
been learning the British variant of it, so it's more formal that
American English :)

2012/5/7 Kris Craig <[email protected]>

>
>
> On Mon, May 7, 2012 at 12:28 AM, Arvids Godjuks <[email protected]>wrote:
>
>> Hello internals,
>>
>> I should voice my opinion that such things like comparing two strings
>> starting with numbers and that they resolve to actual integer/float for
>> comparation is bad, really bad. That just defies the logic and yealds
>> absolutly unexpected results. I pride myself that i know the juggling
>> rules
>> well, but I'm shocked by this to say the least...
>> In my opinion this should change no matter the BC breaks it will create,
>> this one affects security big time. It's good I actually hash my passwrds
>> in the MySQL and not on the PHP side, but I have seen hash comparations
>> with == all the time. And now that this has been discussed in detail I
>> expect this to be used as an attack method grow wide.
>> 07.05.2012 5:32 пользователь "Tjerk Anne Meesters" <[email protected]>
>> написал:
>
>
> Forgive me if I'm missing something, but why are people using == for
> security-sensitive string comparisons (like hashed passwords) in the first
> place?! If you needs something that's safe, isn't that what strcmp() and
> strcasecmp() are for? For my part, I pretty much never use == on string
> comparison, though admittedly that's probably just a matter of having
> having come from a C background before PHP.
>
> That being said, I agree that this *definitely* should be fixed if the
> examples cited are indeed accurate (I've been working with PHP for over 10
> years and I was never aware of this bizarre behavior, either). I don't
> know the history of this, but I at least would consider it to be a bug. A
> rather large one, in fact; though I think some of the fears expressed are a
> bit hyperbolic. And if you're fixing a serious bug or security
> vulnerability, as a general rule of thumb, this automatically supercedes
> any concerns regarding BC breakage IMHO. But if that really is a lingering
> concern, I'd suggest targetting the fix for PHP 6, since people would (or
> at least should) expect that some PHP 5 code may behave differently in PHP
> 6 anyway given that it's a major release
>
> --Kris
>
>
Hartmut Holzgraefe
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 02:30PM
On 05/07/2012 09:28 AM, Arvids Godjuks wrote:
> Hello internals,
>
> I should voice my opinion that such things like comparing two strings
> starting with numbers and that they resolve to actual integer/float for
> comparation is bad, really bad. That just defies the logic and yealds
> absolutly unexpected results. I pride myself that i know the juggling rules
> well, but I'm shocked by this to say the least..

you have to see this in the "web context" where all input from a
HTTP client arrives as strings without type information (and some
database result data comes in as string data, too)

In that context it perfectly makes sense that

"1" == "1.0"

returns true even if both operands are strings.

"123ABF453..." == "123DFEABC..." is a different story though,
and guess what? These are *not* considered equal, at least not
by the 5.3.6 instance on the system i'm currently testing with:

<?php
$a = "123ABF453..."; //a password
$b = "123DFEABC..."; //another one
if ($a == $b){
echo "you're in";
} else {
echo "no, you don't get in *that* easy!";
}
?>

will print "no, you don't get in *that* easy!" just fine

--
hartmut

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Hartmut Holzgraefe
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 02:40PM
On 05/07/2012 05:32 AM, Tjerk Anne Meesters wrote:

> Validated or not, why would type juggling even come into the picture
> if both variables are of the same type?

For the simple reason that web forms return all input as strings, even
if the input is actually meant to be numeric

Many PHP database backend functions also return all result fields
as strings regardless of the actual result type, e.g. mysql_fetch_*(),
mysqli_fetch_*() and pg_fetch_*() (although that's more debatable)

So if both operands look numeric (even though they are actually of
type string) type juggling kicks, and in your MD5 example it
unfortunately kicks in with a conversion to float for both sides
and you're running into the "never compare floats for equality"
trap ... (which is being worked on for the string comparison case
though, i just don't have the bug number at hand right now)

--
hartmut


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Tjerk Anne Meesters
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 07, 2012 08:10PM
> On 05/07/2012 05:32 AM, Tjerk Anne Meesters wrote:
>
>> Validated or not, why would type juggling even come into the picture
>> if both variables are of the same type?
>
> For the simple reason that web forms return all input as strings, even
> if the input is actually meant to be numeric
>
> Many PHP database backend functions also return all result fields
> as strings regardless of the actual result type, e.g. mysql_fetch_*(),
> mysqli_fetch_*() and pg_fetch_*()  (although that's more debatable)

Granted, I realized that my comment came across a bit too generic.
What I meant to say is that the type juggling should be applied within
its realm of usefulness and skipped otherwise lest it tell me "they're
both pretty big numbers" :)

I've always felt overly pedantic when I used === or strcmp() in string
comparisons (assuming the equality edge cases were not an issue; e.g.
same string composition).

>
> So if both operands look numeric (even though they are actually of
> type string) type juggling kicks, and in your MD5 example it
> unfortunately kicks in with a conversion to float for both sides
> and you're running into the "never compare floats for equality"
> trap ... (which is being worked on for the string comparison case
> though, i just don't have the bug number at hand right now)

I'm assuming that you're referring to this bug reference:
https://bugs.php.net/bug.php?id=54547

The proposed patch therein looks okay, though I'm not sure why the
added test case is for 64bit systems only; seems to me that it can /
should be run for other systems too.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Raymond Irving
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 08, 2012 03:30AM
I was very surprised when I came across the == issue sometime ago. IMO
strings should be compared as strings. They should never be converted to
integer.


1=="1" // always convert the number value to a string and then
compare it
"foo" == 0 // should return false

"123abc" == "123nth" // compare as string. Do not convert to numeric values

"0"==0 // true
0=="0." // false

PHP is great but if we can work together to remove the flaws, then we can
make it even greater. We have to leave the past behind us and look at where
we want PHP to be in the next 5 years


Best regards,
Matt Wilson
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 08, 2012 04:10AM
While it's not the prettiest of side effects in php, I don't agree it should be "fixed"

On top of a massive BC break, it's not as if the results are inconsistent. Learning php means learning how type juggling works.

At most, I'd remove the part that truncates numeric strings like "123abc" and always convert them to 0, because that's almost *never* an intended effect. One could argue that "123" == 123 is, however.


On May 7, 2012, at 6:25 PM, Raymond Irving wrote:

> I was very surprised when I came across the == issue sometime ago. IMO
> strings should be compared as strings. They should never be converted to
> integer.
>
>
> 1=="1" // always convert the number value to a string and then
> compare it
> "foo" == 0 // should return false
>
> "123abc" == "123nth" // compare as string. Do not convert to numeric values
>
> "0"==0 // true
> 0=="0." // false
>
> PHP is great but if we can work together to remove the flaws, then we can
> make it even greater. We have to leave the past behind us and look at where
> we want PHP to be in the next 5 years
>
>
> Best regards,


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sanford Whiteman
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 08, 2012 07:20AM
> At most, I'd remove the part that truncates numeric strings like
> "123abc" and always convert them to 0, because that's almost *never*
> an intended effect.

I too find it hard to think of the situation in which data must be
stored, even in a temporary form, as "123abc" but is meant to be
equivalent to 123. But it obviously has happened a bunch after all
these years.

Maybe a non-accidental example is where "123a", 123b", and "123c" are
all expected to be juggled to 123 -- i.e. the code is purposely
utilizing the PHP behavior in place of explicit truncation in order to
group certain items together. I'm 99.999% sure I've never done this,
but it would be devilishly hard to find the bugs that would come from
changing this behavior. Enough that I could see it being a major event
that tarnishes the release of PHP-dot-whatever-makes-this-change.

I also want to point out, as I did the other month when this came up,
that MySQL does autoconversion that is identical to this case [1], and
with **MP pairings so absurdly common, it makes sense to keep this
behavior in PHP for this reason alone.

-- S.


[1] SELECT '123abc' = 123 // true in MySQL (but not in all RDBMSes)





> One could argue that "123" == 123 is, however.


> On May 7, 2012, at 6:25 PM, Raymond Irving wrote:

>> I was very surprised when I came across the == issue sometime ago. IMO
>> strings should be compared as strings. They should never be converted to
>> integer.
>>
>>
>> 1=="1" // always convert the number value to a string and then
>> compare it
>> "foo" == 0 // should return false
>>
>> "123abc" == "123nth" // compare as string. Do not convert to numeric values
>>
>> "0"==0 // true
>> 0=="0." // false
>>
>> PHP is great but if we can work together to remove the flaws, then we can
>> make it even greater. We have to leave the past behind us and look at where
>> we want PHP to be in the next 5 years
>>
>>
>> Best regards,




--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Joe Gillotti
Re: [PHP-DEV] [off] PHP: a fractal of bad design
May 09, 2012 10:22AM
Nothing's stopping you from using === for integer comparison or
validating your integer string using either ctype_digit() or
is_numeric() before comparing it. (The difference between these two
functions is is_numeric() allows for decimal points)

On 05/07/2012 09:25 PM, Raymond Irving wrote:
> I was very surprised when I came across the == issue sometime ago. IMO
> strings should be compared as strings. They should never be converted to
> integer.
>
>
> 1=="1" // always convert the number value to a string and then
> compare it
> "foo" == 0 // should return false
>
> "123abc" == "123nth" // compare as string. Do not convert to numeric values
>
> "0"==0 // true
> 0=="0." // false
>
> PHP is great but if we can work together to remove the flaws, then we can
> make it even greater. We have to leave the past behind us and look at where
> we want PHP to be in the next 5 years
>
>
> Best regards,
>


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login