Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87725
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.180 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <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> <882D42B0-3554-4CAC-9EB9-09A0F00A35E8@craigfrancis.co.uk>
Date: Wed, 12 Aug 2015 08:27:17 +0900
Message-ID: <CAGa2bXaOHoRk5uWei2dPBetfOQqa9bF5OL8vw6sAfN=WB42jzw@mail.gmail.com>
To: Craig Francis <craig@craigfrancis.co.uk>
Cc: Anthony Ferrara <ircmaxell@gmail.com>, Julien Pauli <jpauli@php.net>, Matt Tait <matt.tait@gmail.com>, 
	PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=94eb2c076e8ec07fd6051d11736d
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: yohgaki@ohgaki.net (Yasuo Ohgaki)

--94eb2c076e8ec07fd6051d11736d
Content-Type: text/plain; charset=UTF-8

Hi all,

On Mon, Aug 10, 2015 at 6:57 PM, Craig Francis <craig@craigfrancis.co.uk>
wrote:

> 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 = '{$_GET['colour']}'");
>

This is awful... It seems someone already fixed the doc.


>
>
> 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.
>

I fully agree "prepared statements fix everything" is simply wrong.

To be secure truly, secure API must split command and all others completely
 _and_ command part must be programmer supplied constant/static/validated
string.
(i.e. Perfectly secure API must prohibit command injections at all)
Prepared statement does not satisfy both conditions.

Many APIs, that are considered secure API, are not perfectly secure
because "command part must be programmer supplied constant/static/validated
string" condition is not checked. It's left to developers.

e.g. execl/execv splits command and arguments, only single command is
allowed,
but if "command" is user parameter, it cannot be secure.


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=: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).
>

I don't think the proposal is useless nor ineffective.
Taint system is nice to have, but the proposal does not seem preferable
resolution.
Don't get me wrong. I agree with your discussion overall.

I tend to dislike all or nothing choice for security related
issues especially, nonetheless
"nothing" is my choice this time...

Regards,

P.S. There are many outputs other than SQL. Context aware automatic
escaping/use of
secure API/validation for various outputs would be nice to have. This would
be new API
and wouldn't help users shooting their own foot, though. I don't have
concrete idea now,
but PHP may have features help it.

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--94eb2c076e8ec07fd6051d11736d--