Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87673
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.175 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAEZfaM6jF=iW2Cc=kNxyFZ4B-HES9aH9ywNWzRXvXOx++06Cag@mail.gmail.com>
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>
Date: Fri, 7 Aug 2015 10:29:12 +0900
Message-ID: <CAGa2bXaY6-k3XwDP2aDmqYba7uyyvi0zyzLpgnB8BNCkiWb4gQ@mail.gmail.com>
To: Matt Tait <matt.tait@gmail.com>
Cc: Anthony Ferrara <ircmaxell@gmail.com>, Julien Pauli <jpauli@php.net>, 
	PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a114908848c0923051cae92eb
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)

--001a114908848c0923051cae92eb
Content-Type: text/plain; charset=UTF-8

Hi Matt,

On Thu, Aug 6, 2015 at 12:46 PM, Matt Tait <matt.tait@gmail.com> wrote:

> I'll take a few of your points in turn.
>
> With regards to the fact that not all SQL queries are directly
> parameterizable, this is true. Structural parts of a query, such as table
> names, column names and complex conditions are hard to parameterize with
> "vanilla" prepared statements, and many developers like to abstract some of
> these structural parts of a SQL query into config files, and append
> additional conditional constraints into the query at runtime based on user
> input.
>
> This feature addresses this head on. So long as the structural parts of the
> prepared statement -- that is, table names, database names and column names
> -- are not themselves attacker-controlled (I can't think of a valid reason
> whey they would be), this feature is happy for developers to concatenate
> them into a query string. For example, the following is not detected by the
> feature as dangerous, because the query (whilst dynamically constructed) is
> the linear concatenation of string literals, and so is a safeconst.
>
> $query = "SELECT * from {$config['tablename']} WHERE id=?";
> if(isset($_GET["filterbycolor"]))
>   $query .= " AND color=?";
> do_prepared_statement($query, array("id" => $_GET["id"] "color" =>
> $_GET["color"]));
>

If you would like to prevent SQL injection via "Identifiers", how about
extend prepared query like

$query = "SELECT * from @ WHERE id=?";
if(isset($_GET["filterbycolor"]))
  $query .= " AND color=?";
do_prepared_statement($query, array("id" => $_GET["id"] "color" =>
$_GET["color"]), array($config['tablename']));

where @ indicates identifier placeholder. The char could be anything that
will not violate SQL syntax. This could be implemented in user land as
there is no standard/native API for identifier placeholder.

Even if there is identifier  placeholder, SQL keyword remains.
So to be perfect, you'll need another place holder for SQL keywords.
There is no escaping for SQL keywords and it has to be validation.
e.g. ORDER BY {$_GET['order']}

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--001a114908848c0923051cae92eb--