Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87698
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain craigfrancis.co.uk designates 209.85.212.179 as permitted sender)
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\))
In-Reply-To: <B2B729F9-DD03-4FD7-9AA1-FA35F1F764FE@gmail.com>
Date: Mon, 10 Aug 2015 10:57:37 +0100
Cc: Matt Tait <matt.tait@gmail.com>,
 PHP Internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <882D42B0-3554-4CAC-9EB9-09A0F00A35E8@craigfrancis.co.uk>
References: <CAEZfaM5t3MpVk=rz7JUPdao=qq96J3cowWQmKboqTgLOXLqaCA@mail.gmail.com> <CAMUwpuTJKO+JCWgiVMYRxGDbj1qdhR3Dvoho004KUh_MJoZD4g@mail.gmail.com> <CAAyV7nExkQtPd3Z0hdYp_+iF8ATsLu0itBrXCQX0zntr9y4eCA@mail.gmail.com> <CAEZfaM5kaMLv9y7T0C9E2Xi=AUuHwAFosjd8wGf6QQ120Pi46Q@mail.gmail.com> <CAAyV7nFsqNkbQwCd85p3uAxY4RrPemnqUYktcuT1Xmg50xfdWQ@mail.gmail.com> <CAEZfaM6jF=iW2Cc=kNxyFZ4B-HES9aH9ywNWzRXvXOx++06Cag@mail.gmail.com> <CAAyV7nGEy8fBYPvDqwmP+3FDSEBqjN9=0gfnbWvOSK6A1c6YJw@mail.gmail.com> <B2B729F9-DD03-4FD7-9AA1-FA35F1F764FE@gmail.com>
To: Anthony Ferrara <ircmaxell@gmail.com>,
 Julien Pauli <jpauli@php.net>,
 Yasuo Ohgaki <yohgaki@ohgaki.net>
Subject: Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
From: craig@craigfrancis.co.uk (Craig Francis)

Hi Anthony, Julien, Yasuo,

I have been reading your conversation with great interest.

But I would urge you to see Matts suggestion as a simple addition to the =
language (it's simpler than my suggestion), where his RFC seems to have =
already addressed your concerns (and he seems to have a working proof of =
concept).

Personally I would like to see any one of these solutions being added to =
the core language, as there are far too many PHP programmers making =
ridiculous mistakes that the core language can be (and should be) =
identifying. That said, I am still biased to my suggestion, which also =
tries to consider other problems like XSS.

But anyway...

Take the code below, it is obviously wrong, but it executes without any =
complaints, and unless someone is checking every line of code that is =
written (note: PHP is doing so already), then the developer will just =
move on without thinking anything is wrong:


	http://php.net/manual/en/pdo.exec.php
	$dbh->exec("DELETE FROM fruit WHERE colour =3D =
'{$_GET['colour']}'");


Over the years I've heard many people say things like "use static =
analysis" or "prepared statements fix everything", but I don't think =
these are enough.

You only have to skim read things like the second comment (with 27 up =
votes) on the PDO prepare page to see that these problems are happening =
all the time:


	http://php.net/manual/en/pdo.prepare.php#111458
	SELECT * FROM users WHERE $search=3D:email


So accept that education alone is not enough, and that the basic =
building blocks like prepared statements or ORMs are not enough, and =
look at these proposals and see how they will make the language =
better... because I can assure you, having a simple tainting system that =
can be switched on to show your mistakes, is considerably better than =
what we have today (i.e. nothing... or maybe using some half baked / =
expensive static analysis tool).

Or are you scared that this will highlight the mistakes you have made in =
your own (probably over-complicated) code? as this is why I want this =
feature, because I know I have made mistakes, and with OOP, it is very =
easy todo so (abstractions like ORMs have a tendency to allow for these =
mistakes to creep in extremely easily).

Craig






On 6 Aug 2015, at 23:57, Matt Tait <matt.tait@gmail.com> wrote:

> Thanks for the feedback Anthony,
>=20
> This feature specifically addresses the points you raise; the feature =
allows parameterized queries constructed with structural parts of the =
query inserted from configuration variables, so long as the resulting =
query is a safe-const as defined by this RFC.
>=20
> If you can come up with an example of a query that either (a) is =
vulnerable to a SQL injection attack and this feature wrongly does not =
detect it as such or (b) a query that cannot be expressed using =
parameterized queries where the parameter is a safe-const as defined by =
this RFC, I'd be genuinely interested to take a look.
>=20
> Hope that clarifies,
> Matt
>=20
>> On Aug 6, 2015, at 14:34, Anthony Ferrara <ircmaxell@gmail.com> =
wrote:
>>=20
>> Matt,
>>=20
>>> You are of course welcome to disagree with the overwhelming body of =
security
>>> advice that parameterized queries are the correct, secure way to =
prevent SQL
>>> injection. In that case, you only need to not enable this feature. =
This
>>> feature is off-by-default, and only attempts to help secure =
webapplications
>>> and webdevelopers who do (or are required, for example by PCI =
compliance
>>> standards) to adopt this security-best-practice to ensure that they =
do so
>>> systematically across their entire website.
>>=20
>> You seem to misunderstand me. I'm *not* saying that you shouldn't use
>> parameterized queries. Nor that they are not one of the best tools
>> available. I am simply pointing out that they are not a sure-fire
>> one-stop solution. There is a lot more that goes into it than simply
>> using prepare/bind. As indicated by the two cases I talked about
>> (structural elements not being supported and implementation quirks) =
as
>> well as others (DOS attacks from wildcards in malformed query
>> parameters, type validation, etc). This is not to say that we should
>> avoid PQ, but that we should recognize that it's not enough to just
>> use one thing and forget about everything else.
>>=20
>> Anthony
>>=20
>> PS: w3schools is NOT w3c. And their content is probably the single
>> largest source of security vulnerabilities for PHP, so citing them in
>> a security discussion is more than a little ironic.
>=20
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>=20