Re: PHP 4.0 Bug #7749: addslashes wrong thing to do

On Fri, 10 Nov 2000, james+phpbug@squish.net wrote: > From: james+phpbug@squish.net > Operating system: n/a > PHP version: 4.0.3pl1 > PHP Bug Type: PCRE related > Bug description: addslashes wrong thing to do > > Three bugs. > > preg_replace: > > $text = preg_replace('/(foo(bar)?) is a good word/', > 'wibble', $text); > > Simple enough. How about: > > $text = preg_replace(/'(foo(bar)?) is a good word/e', > > '(length(\'\2\')>0)?"wibble":"wobble"', > $text); > > The first thing to note here is that the idea of substituting into the replacement string like > this was a very bad idea, I would encourage you to phase this out in favour of $<num> > replacement. Could, but then what about backwards compatibility? > The two obvious things that PHP could get wrong with this form of substitution, PHP gets wrong > :-) > > Firstly - when \2 does not exist because there was no match, you should should get > '', infact with PHP you get ^B, it seems you're simply looking for \<nums> that > created matches rather than all \<nums. You're right, it seems like a correct behavior. I fixed it now. > Secondly - as a security-aware person, I immediate recognise the problems that '\1' > could cause. A quick look at the code reveals that (thankfully) some effort is being made to quote > the inserted string (undocumentedly). However, the code in PHP uses addslashes() which was designed > for database use and not internal PHP single-quote escaping. PHP's single-quotes only look for > \' and \\ and therefore the escaping of " to \" and NULL to \0 in addslashes() will > cause spurious backslashes to enter the text. What if the match contains " and your expression looks like 'length("\\2")'? Both " and ' need to be escaped. > On an aside note, I also think it was a bad idea to put delimiters into the search string, > there is no point to this at all and is just a burden to the user. The point was to make it easy for people used to Perl's regex syntax. > PHP does not support all of perl's delimiters, particularly it does not support the (), > {}, [] matching delimiters. This code will not work: > > preg_replace("{wibble}", "wobble", $text); This can be fixed. -Andrei We all have photographic memories, it's just that some of us don't have any film.