Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-31 Thread Yasuo Ohgaki
Hi Matt,

My friend made a product called sql_firewall for PostgreSQL.

https://github.com/uptimejp/sql_firewall
http://pgsnaga.blogspot.jp/2015/08/postgresql-sql-firewall.html

It's not directly relevant to your proposal, but this kind of database extension
could be alternative.

PDO has parser for params. Parser may be extended to parse basic SQL syntax
and store syntax tree hash like sql_firewall and compare it to reject
SQL injection
attacks.

Just FYI.

Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net


On Wed, Jul 29, 2015 at 2:33 AM, Matt Tait  wrote:
> Hi all,
>
> I've written an RFC (and PoC) about automatic detection and blocking of SQL
> injection vulnerabilities directly from inside PHP via automated taint
> analysis.
>
> https://wiki.php.net/rfc/sql_injection_protection
>
> In short, we make zend_strings track where their value originated. If it
> originated as a T_STRING, from a primitive (like int) promotion, or as a
> concatenation of such strings, it's query that can't have been SQL-injected
> by an attacker controlled string. If we can't prove that the query is safe,
> that means that the query is either certainly vulnerable to a SQL-injection
> vulnerability, or sufficiently complex that it should be parameterized
> just-to-be-sure.
>
> There's also a working proof of concept over here:
>
> http://phpoops.cloudapp.net/oops.php
>
> You'll notice that the page makes a large number of SQL statements, most of
> which are not vulnerable to SQL injection, but one is. The proof of concept
> is smart enough to block that one vulnerable request, and leave all of the
> others unchanged.
>
> In terms of performance, the cost here is negligible. This is just basic
> variable taint analysis under the hood, (not an up-front intraprocedurale
> static analysis or anything complex) so there's basically no slow down.
>
> PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
> injections are the result of a developer either not understanding how to
> prevent SQL injection, or taking a shortcut because it's fewer keystrokes
> to do it a "feels safe" rather than "is safe" way.
>
> What do you all think? There's obviously a bit more work to do; the PoC
> currently only covers mysqli_query, but I thought this stage is an
> interesting point to throw it open to comments before working to complete
> it.
>
> Matt

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-13 Thread Craig Francis
 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...




Hi Yasuo,

I can certainly see why you would choose not to... e.g. if magic quotes had not 
been implemented, I think things would have been better.

But I think any one of these 3 proposals (or maybe something else) would 
provide very helpful information to developers.

I don't think anyone wants to go down the PHP fixes things automatically 
route, as I've yet to find any kind of automatic system that works reliably... 
for example I hate tempting systems that try to automatically HTML encode 
values (good in theory, but personally I want them to complain when I get it 
wrong - so I can either fix it, or put something in place to say that this one 
is fine).

But this is a technique that has been proven to work in static analysis 
tools... it's probably one of their main features (even if they will always 
find it hard to check properly - as the code itself isn't really running), 
whereas PHP can be doing these checks as the code is executed.

What PHP does with this information is up for debate, I personally just want a 
simple log of things to fix... and I take Ronabop's point that maybe this 
should not be enabled by default, as so many developers are out there who will 
see it as an annoyance (at least until the feature is introduced to them 
properly).

I've also added a couple of notes below.

Craig







On 12 Aug 2015, at 00:27, Yasuo Ohgaki yohg...@ohgaki.net wrote:

 Hi all,
 
 On Mon, Aug 10, 2015 at 6:57 PM, Craig Francis cr...@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.
  



Actually, this was me demonstrating what I've seen a couple of developers do 
before now... take the example from the documentation, then hack it to make it 
work as they want it to (without reading anything else on the page).




 
 
 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.
 



Fair point... but I think we should still support escaping (kind of necessary 
for HTML output).

But Matt's suggestion works with your preferred setup, where the main SQL 
command is made up of constants defined in the PHP code itself (i.e. no 
variables from outside).

So if we had a way to check the source of a variables content, then functions 
like mysqli_query() or a wrapper for shell_exec() can check that the main 
command has only come from string constants, anything variable related would be 
passed in separately.




 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 
 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-13 Thread Craig Francis
On 12 Aug 2015, at 00:43, Christoph Becker cmbecke...@gmx.de wrote:

 On 10.08.2015 at 11:57, Craig Francis wrote:
 
 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
 
 Skim reading things might be the problem (here).  The user contributed
 note states:
 
 | In my case I allow the user to enter their username or email,
 | determine which they've entered and set $search to username or
 | email. As this value is not entered by the user there is no
 | potential for SQL injection and thus safe to use as I have done.
 
 So to me that note looks pretty fine.



But that is the problem, many programmers (and I know I don't have numbers to 
back this up) do just skim read the docs. They often have a problem, and do 
little research to solve that immediate problem (i.e. make it run, don't care 
what it does or how it does it).

I say this as someone who is frequently finding issues that just should not be 
happening. But at the moment there is nothing that helps developers identify 
those problems or mistakes (with the possible exception of static analysis).

Craig


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-11 Thread Yasuo Ohgaki
Hi all,

On Mon, Aug 10, 2015 at 6:57 PM, Craig Francis cr...@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
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-11 Thread Christoph Becker
On 10.08.2015 at 11:57, Craig Francis wrote:

 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

Skim reading things might be the problem (here).  The user contributed
note states:

| In my case I allow the user to enter their username or email,
| determine which they've entered and set $search to username or
| email. As this value is not entered by the user there is no
| potential for SQL injection and thus safe to use as I have done.

So to me that note looks pretty fine.

-- 
Christoph M. Becker

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-10 Thread 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 = '{$_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=: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.t...@gmail.com wrote:

 Thanks for the feedback Anthony,
 
 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.
 
 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.
 
 Hope that clarifies,
 Matt
 
 On Aug 6, 2015, at 14:34, Anthony Ferrara ircmax...@gmail.com wrote:
 
 Matt,
 
 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.
 
 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.
 
 Anthony
 
 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.
 
 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php
 


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-06 Thread Anthony Ferrara
Matt,

 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.

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.

Anthony

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.

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-06 Thread Yasuo Ohgaki
Hi Matt,

On Thu, Aug 6, 2015 at 12:46 PM, Matt Tait matt.t...@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
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-06 Thread Yasuo Ohgaki
On Fri, Aug 7, 2015 at 10:29 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:

 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']}


Oops the last line should be
e.g. ORDER BY col {$_GET['order']}

BTW, instead of improving PHP, users are better to request identifier
escape API
to DB developers like PQescapeIdentifier() in PostgreSQL's client library.
IMO.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-06 Thread Matt Tait
Thanks for the feedback Anthony,

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.

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.

Hope that clarifies,
Matt

 On Aug 6, 2015, at 14:34, Anthony Ferrara ircmax...@gmail.com wrote:
 
 Matt,
 
 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.
 
 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.
 
 Anthony
 
 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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-06 Thread Dennis Birkholz
Hi Matt,

Am 06.08.2015 um 05:46 schrieb Matt Tait:
 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.

and how exactly do you guarantee the structural part from a
configuration is not attacker-controlled? The config may come from a
json file, from a database, from APCu, etc. and may have been
inserted/generated previously from unsafe user input. If I understand
you right, these parts will not be tainted and thus will give a wrong
feeling of safety to the unskilled/unknowing programmer. Or did I miss
that part in the RFC?

Greets
Dennis

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-05 Thread Matt Tait
Thanks for your feedback, Anthony.

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]));

We can even take this to an extreme to prove how flexible this can be.
Here's a function that issues a SQL statement where everything from the
table name and column name, to the structure and form of the condition, as
well as the limit, order and direction of the output are all chosen by the
caller. So long as the structural parts (i.e. the table names, database
names and column names) are safeconsts, the resulting query is a safeconst,
and this feature correctly identifies the resulting query as untainted).

http://phpoops.cloudapp.net/dynamic_pdo.php?viewcode
http://phpoops.cloudapp.net/dynamic_pdo.php

Finally, this feature is off-by-default. This means that that the feature
only helps developers who want to (or are required to) parameterize their
queries. It detects queries where the query cannot be proven safe, and
then informs the developer that they need to upgrade the query so that the
dynamic parts of the query are parameterized to be in compliance with the
website's PHP.INI settings for this feature.

==

With regard to static analysis, I have a lot of respect for static analysis
tools, but especially in the webapplication world, they lack adoption, and
hence have limited real-world value. In over a decade of security
consulting experience, and with the notable exception of Google and
Microsoft, I've never seen any company that routinely scanned their website
with static analysis tools more complex than grep.

As well as poor adoption, static analysis tools quickly run into
intractible problems doing analysis over unbound-loops, indirect function
calls or eval statements, and can get confused when looking at files out of
context (e.g. scanning included files). Runtime analysis, on the other
hand, always runs, has zero startup cost, negligble runtime cost and
guarrantees to check every codepath that is actually taken; regardless of
how it was reached.

For this reason, I think static analysis is the wrong tool for this job.

==

With regard to SQL parameterization not always being safe, the case you are
citing is due to a security defect in the way PHP emulates SQL
parameterization for old versions of MySQL that don't support
parameterization. In this particular case, PHP emulates the
parametization by escaping the parameters, and -- since escaping is a
dangerous way to build SQL query statements -- this goes horribly wrong and
leads to a SQL injection. But to be clear: this is a defect in PHP's
/emulation/ of prepared statements when real prepared statements are not
available, not a problem of prepared statements themselves.

Where parameterized queries are available at the database layer, and PHP is
not defectively emulating them, they are (by definition) immune from SQL
injection. Parameterized queries are not flattened to a string using
escaping. They are sent to the database in parameterized form, and are
replaced at AST-construction stage in the database itself. This means they
are immune from weird bugs like character-set differences between
application and SQL-server: so long as the query is untainted, the attacker
cannot affect the structure of the AST using only parameter values, and
hence can never SQL-inject the database.

To quote Wikipedia's article on prepared statements (
https://en.wikipedia.org/wiki/Prepared_statement):
 Prepared statements are resilient against SQL injection, because
parameter values, which are transmitted later
 using a different protocol, need not be correctly escaped. If the
original statement template is not derived
 from external input, SQL injection cannot occur.

To quote the W3C (http://www.w3schools.com/sql/sql_injection.asp)
 The only proven way to protect a web site from SQL injection attacks, is
to use SQL parameters.

To quote security consortium 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-05 Thread Julien Pauli
On Tue, Jul 28, 2015 at 7:33 PM, Matt Tait matt.t...@gmail.com wrote:

 Hi all,

 I've written an RFC (and PoC) about automatic detection and blocking of SQL
 injection vulnerabilities directly from inside PHP via automated taint
 analysis.

 https://wiki.php.net/rfc/sql_injection_protection

 In short, we make zend_strings track where their value originated. If it
 originated as a T_STRING, from a primitive (like int) promotion, or as a
 concatenation of such strings, it's query that can't have been SQL-injected
 by an attacker controlled string. If we can't prove that the query is safe,
 that means that the query is either certainly vulnerable to a SQL-injection
 vulnerability, or sufficiently complex that it should be parameterized
 just-to-be-sure.

 There's also a working proof of concept over here:

 http://phpoops.cloudapp.net/oops.php

 You'll notice that the page makes a large number of SQL statements, most of
 which are not vulnerable to SQL injection, but one is. The proof of concept
 is smart enough to block that one vulnerable request, and leave all of the
 others unchanged.

 In terms of performance, the cost here is negligible. This is just basic
 variable taint analysis under the hood, (not an up-front intraprocedurale
 static analysis or anything complex) so there's basically no slow down.

 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.

 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.


I haven't read all the answers to the thread, but the RFC.

What fools me, is that you want to patch an internal, highly critical used
concept of the engine : zend_string, just for one use case.

Everything touching SQL should be independant from the engine.
Have a look at ext/mysqlnd, that plays with strings and builds a parser on
top of them to analyze SQL queries. Same for PDO.

I think such a concept should not be engine global.

Also, patching zend_string will break ABI, and should then not be merged
until next major, aka PHP8.
We have now a stable code base, PHP7 is beta, no more ABI breakage and
every extension recompilation please.

PHP has always been an extension based language : embed your thoughts into
extensions, and prevent from hacking the global engine if it is not to
support a global idea used everywhere.

Also, we have ext/tainted ; and back in 2006 when PHP5.2 were released,
such ideas as SQL injection prevention into the engine, were abandonned
because too specific. We concluded by thinking about PHP as being a general
purpose language, and high level security such as SQL injection prevention
should be taken care of in userland, or self-contained in extensions.


My thoughts.


Julien.Pauli


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-05 Thread Anthony Ferrara
All,

On Wed, Aug 5, 2015 at 10:40 AM, Julien Pauli jpa...@php.net wrote:
 On Tue, Jul 28, 2015 at 7:33 PM, Matt Tait matt.t...@gmail.com wrote:

 Hi all,

 I've written an RFC (and PoC) about automatic detection and blocking of SQL
 injection vulnerabilities directly from inside PHP via automated taint
 analysis.

 https://wiki.php.net/rfc/sql_injection_protection

 In short, we make zend_strings track where their value originated. If it
 originated as a T_STRING, from a primitive (like int) promotion, or as a
 concatenation of such strings, it's query that can't have been SQL-injected
 by an attacker controlled string. If we can't prove that the query is safe,
 that means that the query is either certainly vulnerable to a SQL-injection
 vulnerability, or sufficiently complex that it should be parameterized
 just-to-be-sure.

 There's also a working proof of concept over here:

 http://phpoops.cloudapp.net/oops.php

 You'll notice that the page makes a large number of SQL statements, most of
 which are not vulnerable to SQL injection, but one is. The proof of concept
 is smart enough to block that one vulnerable request, and leave all of the
 others unchanged.

 In terms of performance, the cost here is negligible. This is just basic
 variable taint analysis under the hood, (not an up-front intraprocedurale
 static analysis or anything complex) so there's basically no slow down.

 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.

 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.


 I haven't read all the answers to the thread, but the RFC.

 What fools me, is that you want to patch an internal, highly critical used
 concept of the engine : zend_string, just for one use case.

 Everything touching SQL should be independant from the engine.
 Have a look at ext/mysqlnd, that plays with strings and builds a parser on
 top of them to analyze SQL queries. Same for PDO.

 I think such a concept should not be engine global.

I agree with Julien here. There are simply too many permutations of
possible tainted destinations. And too many ways of verifying if
something is actually secure or not.

For example, would taint checking detect ctype_digit() guards? Or
filter_* guards?

But even deeper than that is the fact that there is not just one thing
that a variable could be tainted for. A non-exhaustive list:

- MySQLi
- Postgres
- PDO
- shell_exec
- HTML context output (XSS)
- XML
- URL encoding
- Shell output (STDOUT)
- JavaScript (using MongoDB, etc)
- etc

And there's an *even deeper* problem. Consider the following code:

$name = $_GET['name'];
$escapedName = mysqli_real_escape_string($conn1, $name);
mysqli_query($conn2, SELECT * FROM users WHERE name='$escapedName' LIMIT 1);

Unless you're tieing the taint to specific connections, that is
*possible* to inject under certain circumstances.

Given the complexities, unless a solution can address at least *some*
of these (beyond the trivial cases) I don't know if it belongs in
core.

 Also, patching zend_string will break ABI, and should then not be merged
 until next major, aka PHP8.
 We have now a stable code base, PHP7 is beta, no more ABI breakage and
 every extension recompilation please.

 PHP has always been an extension based language : embed your thoughts into
 extensions, and prevent from hacking the global engine if it is not to
 support a global idea used everywhere.

 Also, we have ext/tainted ; and back in 2006 when PHP5.2 were released,
 such ideas as SQL injection prevention into the engine, were abandonned
 because too specific. We concluded by thinking about PHP as being a general
 purpose language, and high level security such as SQL injection prevention
 should be taken care of in userland, or self-contained in extensions.

Now, if you can create a framework for tainting variables that
extensions can hook into, that's something interesting. Because now
extensions can declare their own taint criteria and requirements, and
have the engine track them. Doing this without sacrificing performance
is going to be tricky, but that would be interesting.

Anthony

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-05 Thread Matt Tait
Thanks for the feedback Anthony and Julien,

The case you refer to using mysqli_real_escape_string is addressed in the
RFC, and cannot be injected when this feature is enabled, as the query is
always marked as tainted and always blocked, regardless of the connection.

Here's your example running on my server:

Source code: http://phpoops.cloudapp.net/example.php?viewcode
Run: http://phpoops.cloudapp.net/example.php

To be clear: this feature does not track taint through escape functions,
regular expression filters, ctype_filters and the like by design. Security
best-practice and more than a decade of security consulting experience show
that developers who rely on filters and escaping rarely manage to do so
competently and systematically enough to prevent SQL-injection on
non-trivial websites. Rather, this feature recognizes that SQL-injection is
already a solved problem. It is uncontroversial and overwhelmingly accepted
security best-practice to ensure that every dynamic SQL query is executed
via a prepared statement rather than via string-escaping.

Consequently, the Zend part of this feature only tracks whether a string is
a string literal, or linear concatenation of string literals (which is
called safeconstness in the RFC). If a safeconst string is sent to
/any/ SQL function in any SQL extension either as the query or prepared
statement, that query is not SQL-injectable, since attackers cannot control
the string literals in your code. If the raw-query or prepared-query is
/not/ a safeconst string, the feature (when enabled) alerts the developer
that the query is too complicated to prove is not injectable, and is a good
candidate to be upgraded to a parameterized SQL statement. When the
feature is in blocking mode, PHP will also block the query from executing.

This feature works best for big websites that are following (or in the
process of migrating their code to follow) this security-best-practice, as
it allows them to systematically verify during development that the query
string to every SQL statement is guaranteed untainted by attackers. Any
refactoring which exposes unparameterized queries to attacker-controlled
data, that SQL query written by that intern who didn't understand
parameterized queries and just concatenated a $_GET variable into a
prepared query string, and any backend services that might be vulnerable to
blind-sql injection will now all be identified as queries that need to be
upgraded to use parameterized SQL.

Developers and website administrators who are confident that their entire
website only uses SQL statements safely can then enable the feature in
blocking mode for their production website. If an attacker finds a path
to a vulnerable SQL statement in their website (maybe hidden in some PDF
extension, or that debug page you accidentally left in the /images/
directory, or just a code-path that was never adequately tested), the
tainted SQL statement is aborted to guarantee that no tainted query string
is every issued against the database on any page across the entire website.

In terms of whether this RFC is too coupled with SQL extensions, I am
sensitive to these concerns, but I don't think they apply for three broad
reasons:

Firstly, the taint logic tracks safeconstness (i.e. whether a zend_string
is a string literal or linear concatenation of string literals), and
by-design does not attempt to interact with escaping functions because (as
your example clearly shows), escaping functions only /sometimes/ prevent
injections, whereas parameterized queries /always/ stop injections. For
this reason the taint feature can be trivially applied to any builltin
function where one parameter is structural and should be constant. It could
be deployed without modification to prevent injection into regular
expression queries for example to prevent denial-of-service, or to prove
that the parameter to shell_exec is constant.

Secondly, this feature applies globally to all SQL extensions, not just
mysqli. This feature encompasses protection of mysqli, PDO and Postgres. In
future, I would like to build this feature out to include some of the other
categories of injection later, but given that SQL-injections remain the
single most commonly exploited vulnerability in PHP applications, I think
starting with them is probably best. Tackling SQL-injection in PHP is a big
enough task as it is, and is a good place to start.

Thirdly, although SQL extensions are extensions, this is a technicality, in
much the same way that Zend Core is not supposed to be coupled to PHP. In
reality, /most/ non-trivial PHP websites use one SQL extension or another
to interact with a database, and despite parameterized queries existing for
over a decade, huge numbers of PHP websites remain vulnerable to them --
with a devastating impact on the company and their users when the
vulnerability is exploited.

Understanding SQL injections is hard for many junior developers to
understand, and hard for companies to apply 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-05 Thread Anthony Ferrara
Matt,

 To be clear: this feature does not track taint through escape functions,
 regular expression filters, ctype_filters and the like by design. Security
 best-practice and more than a decade of security consulting experience show
 that developers who rely on filters and escaping rarely manage to do so
 competently and systematically enough to prevent SQL-injection on
 non-trivial websites. Rather, this feature recognizes that SQL-injection is
 already a solved problem. It is uncontroversial and overwhelmingly accepted
 security best-practice to ensure that every dynamic SQL query is executed
 via a prepared statement rather than via string-escaping.

Except that not every part of a query is preparable. And many
non-trivial complexity websites use these portions of queries
extensively.

Hence a blanket **programming language level** blocking of it is not
really going to fly.

 This feature works best for big websites that are following (or in the
 process of migrating their code to follow) this security-best-practice, as
 it allows them to systematically verify during development that the query
 string to every SQL statement is guaranteed untainted by attackers.

Actually, this allows them to verify at runtime. Not during
development (codepaths that aren't hit at devtime can still cause
problems at runtime).

If this is your goal, I'd suggest doing static analysis to prove (or
not) it. I have a working proof-of-concept of one here:
https://github.com/ircmaxell/php-security-scanner

 In terms of whether this RFC is too coupled with SQL extensions, I am
 sensitive to these concerns, but I don't think they apply for three broad
 reasons:

 Firstly, the taint logic tracks safeconstness (i.e. whether a zend_string
 is a string literal or linear concatenation of string literals), and
 by-design does not attempt to interact with escaping functions because (as
 your example clearly shows), escaping functions only /sometimes/ prevent
 injections, whereas parameterized queries /always/ stop injections.

No they don't. They only stop injections when used correctly. For
example (an edge case, but an important one):
http://stackoverflow.com/questions/134099/are-pdo-prepared-statements-sufficient-to-prevent-sql-injection/12202218#12202218

And that doesn't even touch on the DB backends that don't support PS.
And it doesn't even touch on the fact that not every dynamic part of a
query is preparable.

 reason the taint feature can be trivially applied to any builltin function
 where one parameter is structural and should be constant. It could be
 deployed without modification to prevent injection into regular expression
 queries for example to prevent denial-of-service, or to prove that the
 parameter to shell_exec is constant.

 Secondly, this feature applies globally to all SQL extensions, not just
 mysqli. This feature encompasses protection of mysqli, PDO and Postgres. In
 future, I would like to build this feature out to include some of the other
 categories of injection later, but given that SQL-injections remain the
 single most commonly exploited vulnerability in PHP applications, I think
 starting with them is probably best. Tackling SQL-injection in PHP is a big
 enough task as it is, and is a good place to start.

 Thirdly, although SQL extensions are extensions, this is a technicality, in
 much the same way that Zend Core is not supposed to be coupled to PHP. In
 reality, /most/ non-trivial PHP websites use one SQL extension or another to
 interact with a database, and despite parameterized queries existing for
 over a decade, huge numbers of PHP websites remain vulnerable to them --
 with a devastating impact on the company and their users when the
 vulnerability is exploited.

Except that there are a number of DB extensions that don't ship with
PHP. And those that aren't shipped at all (they are custom propriatary
extensions). And those that aren't really SQL, but use similar
semantics (MongoDB).

 Understanding SQL injections is hard for many junior developers to
 understand, and hard for companies to apply systematically. The impact of
 them getting it wrong can be catastrophic to companies and is a major threat
 to users' online privacy when their data gets compromised. I think having
 PHP give developers a hand to write code that is unambiguously safe from
 hackers would be a good for whole PHP community.

It's hard, because tutorials make it hard. It's hard because we show
concatenation as the right way of doing things, and then tell them
it's wrong. It's hard, because we suck at teaching.

Kent Beck had an awesome passage in his book Test-Driven-Development
By Example. I'll paraphrase here, but he said that every engineer he
worked with found testing difficult. However, his daughter found it
easy and never even thought about it. It was because she was taught
TDD from the ground up. She was taught that you don't write code and
then test, you just test  write code. So she never had time to learn
bad 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-08-02 Thread Craig Francis
On 31 Jul 2015, at 17:40, Ronald Chmara rona...@gmail.com wrote:
 The RFC and bug report both make an erroneous assumption that unescaped GPC 
 input is wrong.




Hi Ronabop,

Slightly continuing our discussion, but also replying to your on-list message...


Starting with your examples:

1) Web applications that send variables directly to the database (e.g. 
phpMyAdmin), if I remember their codebase correctly, there is about 3 places 
where that happens, and after doing an authorisation and CSRF check, they could 
put in the 1 line to say that this value is already SQL escaped.

2) Overhead of escaping, where you later explained that this was more for 
legacy systems such as FoxBase and legacy ODBC  data sources, where they pass 
the values over the network to have it escaped (slow)... and while that is a 
valid concern, hopefully all escaping is done on the client side now (if not, 
my previously non-parameterised queries would have taken far too long to 
load)... but that said, mysql_real_escape_string() isn't good enough either (it 
does not apply quote marks, as Matts example shows), hence why my examples talk 
about pg_escape_literal().

3) Generic user written escaping libraries/functions to support multiple 
destinations... if they are still using the native escaping functions, then 
they don't need to do anything different... if the are using something like 
addslashes (maybe this is acceptable for something), then that library/function 
should mark it as having been escaped.

4) From a pre-cleaned source, e.g. taking HTML from a WYSIWYG editor, passing 
it though HTMLTidy, storing in the database... likewise, this just needs a 
single function call to say that this value is already HTML encoded - this is 
the bio_html example shown in bug 69886 :-)

5) Where a developer is doing a file read off of a local hard drive and 
assuming their file contents will never have an SQL injection... well, I'd 
like all string variables (e.g. from GPC, a file, database, xml, etc) to start 
with the assumption that it is unescaped... the developer could mark that as 
having been escaped, but more likely, just escape it (or use it with a hard 
coded, parameterised query, in the case of SQL).

--

But I completely agree that raising too many notices will just mean that this 
feature would be switched off.

I've been focusing too much of my own systems, and the assumption that most 
systems would/should be using parameterised queries, html encoding, etc (so 
they shouldn't get any notices, unless they have made a couple of mistakes).

So the suggestion of using a new ini setting is a good idea (from Matts or the 
2008 RFC's)... or as you suggested, perhaps have it on a per file basis, that 
could also work, like the declare(strict_types=1).

That way we have a useful security feature that can be switched on (rather than 
everyone investing in an expensive static code analysis system, which uses dark 
magic to find every problem)... then hopefully early adopters would start 
using it, it slowly becomes good practice, and sometime in the long distant 
future, it can be enabled by default (while in the mean time we try to educate 
as many developers as we can though channels like Stack Overflow).

---

As to the rarely comment, fair point, I don't have numbers to back this up.

What I'm trying to say is that, for the typical use case for websites (i.e. not 
things like phpMyAdmin), when you are using values in SQL, it is for them to be 
used as variables in the query... e.g. where an id equals this value, or insert 
a record with these values, or update this record with this value... i.e. how 
values in ORMs should be used.

Likewise, when you have variables that need to be printed in HTML, the safe 
normal would be to have the PHP (or other static code) provide the HTML, and 
the variables should be html encoded (e.g. if you were printing someones 
name)... there is an exception to this was explained above (example 4).

I realise that I'm trying to word it differently, but what I'm trying to say is 
that all values should be assumed bad, and it is up to the developer to either 
escape them, use parameterised queries, etc... or call a single function to say 
yes, I know this one is fine... then PHP can identify anything that has been 
missed.

Craig







On 31 Jul 2015, at 17:40, Ronald Chmara rona...@gmail.com wrote:

 On Thu, Jul 30, 2015 at 8:38 AM, Craig Francis cr...@craigfrancis.co.uk 
 wrote:
 On 30 Jul 2015, at 16:26, Ronald Chmara rona...@gmail.com wrote:
  Perhaps I have missed something in this discussion
 I think you have... my email from a couple of weeks ago was ignored... so I 
 replied to Matt's suggestion (which is similar, but different).
 Please, just spend a few minutes reading my suggestion, it has absolutely 
 nothing todo with breaking applications:
http://news.php.net/php.internals/87207
https://bugs.php.net/bug.php?id=69886
 
 The RFC and bug report both make an erroneous assumption 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Craig Francis
On 31 Jul 2015, at 15:00, Joe Watkins pthre...@pthreads.org wrote:

 Even the best implementation need only have a single hole in it and 
 everything falls apart, one extension not doing something properly, or one 
 mistake in the implementation ... it's not hard to imagine these things 
 happening, right ?




Hi Joe,

I've just replied to Matt and your email came in as I sent... so a bit of a 
change there.

But what I was proposing was that PHP simply checks the way in which the 
variables (strings) are being passed around... if something from $_GET is 
passed into mysqli_query without being escaped (as a parameter or as a quoted 
escaped string), it will still continue, but a note will be written to the log 
(or on the page if display_errors is on).

The same would happen when you echo a $_GET variable :-)

So in 3015, if the taint checking isn't configured properly (switched on)... it 
wouldn't change the execution, it's just not checking things (oh well), it 
should still have been coded properly to begin with.

So the way in which PHP executes is not effected, it's just picking up the oh, 
you probably shouldn't be doing this.

Now Matt's suggestion allowed things to be actively blocked... I'm not sure I'd 
use that, but it might work for some (I'd just be happy having a log to check, 
in the same way that I use the logs for undefined variables, script timeouts, 
etc).

And yes, you won't get a perfect system (there are some edge cases)... but 
keeping it as simple as possible (hence why I want to take a slightly different 
approach to the 2008 RFC), it should pick up the most common mistakes.

But please do talk about security, you may be wrong, but thats fine, I'm sure 
someone will be able to correct you... I personally feel that too many people 
try to ignore security, and that's why we keep having so many problems (that 
said, performance and accessibility also need some attention, but that's more 
for the developers creating the websites, rather than PHP internals).

Craig







On 31 Jul 2015, at 15:00, Joe Watkins pthre...@pthreads.org wrote:

 Morning Craig,
 
 I think Pierre and I object to the concept, regardless of the intricacies 
 of any particular implementation. 
 
 Even the best implementation need only have a single hole in it and 
 everything falls apart, one extension not doing something properly, or one 
 mistake in the implementation ... it's not hard to imagine these things 
 happening, right ?
 
 When I made reference to safe_mode, it wasn't because I misunderstood the 
 concept, I know you're not introducing another safe mode, but introducing a 
 INI, or build, or configuration option that says we got this is doomed to 
 fail whatever.
 
 I don't like to talk about security, it's not something I feel qualified 
 to talk about really, but I can observe that catch-all security features 
 have already been proven to fail ... 
 
 It's the year 3015, we're all running PHP442, our taint implementation is 
 flawless, every major framework (which we have a futurish word for) relies on 
 tainting and thinks SQLi is no longer a thing to worry about, because we got 
 this. A junior sysadmin, on mars, is charged with upgrading PHP on the 
 server that is responsible for crop irrigation on the terra-formed world, and 
 forgets to configure taint properly. The upgrade goes live, renegade ruby 
 fans hack the system disrupting food production, leading to the eventual 
 death of everything.
 
It's Friday afternoon, and that was a bit of fun ... I hope I lifted your 
 mood ... push on, I can be wrong, Pierre can be wrong, we can all be wrong ...
 
 Cheers
 Joe
 
 On Fri, Jul 31, 2015 at 11:40 AM, Craig Francis cr...@craigfrancis.co.uk 
 wrote:
 On 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote:
 
 Even if some of those people replying haven't read or don't understand what 
 you are suggesting, it is not a good tactic to assume that and reply with 
 read the RFC.
 
 
 
 Hi Joe,
 
 Sorry about yesterday, I have done as you have said, and I have read those 
 responses again, but unfortunately I still feel that I was right in my 
 assumptions (notes below, maybe some interesting additions?).
 
 In general I have been getting very frustrated that no-one seems to really 
 care about security (and to be fair, it has been annoying me for far too many 
 years).
 
 Keeping in mind that I work with other developers who routinely keep 
 introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying things 
 like switching the CSP off, because they copy/paste some hideous/insecure JS, 
 and can't be bothered to work out why the eval function isn't working.
 
 So maybe I should start a new thread, without Matt's subject (btw Matt, I 
 really appreciate what you are trying todo, I disagree with the blocking 
 element, and I think we can also address more than just SQL injection 
 vulnerabilities)... maybe something like I've found this one weird trick 
 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Matt Tait
Thanks for the feedback Joe!

To be clear, this feature is not an alternative to coding SQL securely with
parameterized queries. Indeed, it's entire purpose is to help developers
move dangerously coded queries (i.e. ones that are not parameterized and
are dynamically constructed using parameters that attackers might control,
like $_GET[], or the output of other database queries) to ones that are
clearly and unambiguously secure, by parameterizing their queries.

This feature is not a replacement for the security guidance of
parameterize all your queries; but it does allow webapplication
developers who follow this guidance to enforce it at the language level to
ensure that they don't let a single SQL-injectable query through the net.

Enabling this feature encourages developers to write code that is secure
even when this feature is disabled.

Hope that clarifies!
Matt

On 31 July 2015 at 15:00, Joe Watkins pthre...@pthreads.org wrote:

 Morning Craig,

 I think Pierre and I object to the concept, regardless of the
 intricacies of any particular implementation.

 Even the best implementation need only have a single hole in it and
 everything falls apart, one extension not doing something properly, or one
 mistake in the implementation ... it's not hard to imagine these things
 happening, right ?

 When I made reference to safe_mode, it wasn't because I misunderstood
 the concept, I know you're not introducing another safe mode, but
 introducing a INI, or build, or configuration option that says we got
 this is doomed to fail whatever.

 I don't like to talk about security, it's not something I feel
 qualified to talk about really, but I can observe that catch-all security
 features have already been proven to fail ...

 It's the year 3015, we're all running PHP442, our taint implementation
 is flawless, every major framework (which we have a futurish word for)
 relies on tainting and thinks SQLi is no longer a thing to worry about,
 because we got this. A junior sysadmin, on mars, is charged with
 upgrading PHP on the server that is responsible for crop irrigation on the
 terra-formed world, and forgets to configure taint properly. The upgrade
 goes live, renegade ruby fans hack the system disrupting food production,
 leading to the eventual death of everything.

It's Friday afternoon, and that was a bit of fun ... I hope I lifted
 your mood ... push on, I can be wrong, Pierre can be wrong, we can all be
 wrong ...

 Cheers
 Joe

 On Fri, Jul 31, 2015 at 11:40 AM, Craig Francis cr...@craigfrancis.co.uk
 wrote:

 On 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote:

 Even if some of those people replying haven't read or don't understand
 what you are suggesting, it is not a good tactic to assume that and reply
 with read the RFC.



 Hi Joe,

 Sorry about yesterday, I have done as you have said, and I have read
 those responses again, but unfortunately I still feel that I was right in
 my assumptions (notes below, maybe some interesting additions?).

 In general I have been getting very frustrated that no-one seems to
 really care about security (and to be fair, it has been annoying me for far
 too many years).

 Keeping in mind that I work with other developers who routinely keep
 introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying
 things like switching the CSP off, because they copy/paste some
 hideous/insecure JS, and can't be bothered to work out why the eval
 function isn't working.

 So maybe I should start a new thread, without Matt's subject (btw Matt, I
 really appreciate what you are trying todo, I disagree with the blocking
 element, and I think we can also address more than just SQL injection
 vulnerabilities)... maybe something like I've found this one weird trick
 that will fix every security issue... sorry, I hate that kind of approach,
 but if it gets a response, maybe its worth it :-)

 Craig



 --
 http://news.php.net/php.internals/87346
 From: Matt Tait
 Reply: N/A

 Original suggestion.

 --
 http://news.php.net/php.internals/87348
 From: Rowan Collins
 Reply: Matt Tait

 Suggestion to Matt to look for previous RFCs.

 --
 http://news.php.net/php.internals/87350
 From: Christoph Becker
 Reply: Matt Tait

 Points out the existing Taint RFC from 2008, by Wietse Venema.

 --
 http://news.php.net/php.internals/87355
 From: Pierre Joye
 Reply: Matt Tait

 Pointing out that there is more than SQLi (true), and it might be an
 impossible task (I disagree, as explained later).

 --
 http://news.php.net/php.internals/87361
 From: Thomas Bley
 Reply: Matt Tait

 Suggesting the use of bound parameters / prepared statements, which I
 agree is how it should be coded by the website developers, but I think the
 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Matt Tait
 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications. If Google has information that indicates that SQLi is
 still more prevalent than XSS, I'd love to see this data.

Thanks Scott!

You are absolutely correct. SQL injection is a solved problem using
parameterized SQL. In fact, parameterizing queries to avoid SQL injection
is so uncontroversially accepted security advice, that this RFC *enforces* the
advice at the language level (unless the website administrator explicitly
disables the protection across the website via PHP.INI).

Examples of SQL queries that *would* and *would not *be blocked by this
tool are given in the RFC. In your case, ORDER BY {$column} ASC would not
be blocked by this tool if $column came from within the PHP application,
but would be blocked if it came from, say, $_GET[column].

Sadly, although SQL injection is a solved problem by using prepared
statements, not everyone follows the advice. Prior to working at Google, I
worked with a company called iSEC Partners who do application review for
various companies. From my experience, between a third and a half of all
the often-huge PHP websites I security-reviewed were vulnerable to SQL
injection somewhere. It wasn't uncommon for the company to have ~5000 or so
SQL queries in their PHP code, only 2-3 of which were vulnerable to
injection. But attackers only need one to ruin your company and steal your
users' data.

To take apart some of the other valid comments on this thread:

== SQL injection is dead ==
This is objectively not the case
https://www.exploit-db.com/search/?action=searchdescription=SQL+injection

== Won't this break every website? ==
No. In its initial release, this feature will be released in disabled
mode. Website owners with heightened security requirements will be able to
upgrade this to logging or blocking mode. This means this feature will
have zero impact on websites that do not explicitly enable it.

== We should be looking at XSS instead ==
Solving SQL injection with parameterized SQL queries is uncontroversial and
universally accepted security advice. Lots of different (some working, some
not) advice for solving XSS exist, but this makes it harder to standardize
one of those ideas into the language without significant controversy.

For example, one website may choose to store HTML content in their
database. This should be printed out to HTML verbatim. Other websites may
accidentally store HTML content in their database, leading to stored-XSS.
The language will not be able to distinguish these two use-cases.

In contrast, taking content out of a database and gluing it into a SQL
statement is /always/ an error, as it can lead to second-order and
blind-SQL injection vulnerabilities. For this reason, this RFC will
initially only be looking at securing websites against SQL-injections,
although perhaps there is scope to also look at XSS-injection
vulnerabilities in a different RFC using similar tools at a later date.

== This breaks applications that use OOP or other complicated ways of
sending data to a database ==
This is not the case. This RFC doesn't use taint analysis to find SQL
injections; it uses reverse-taint analysis to avoid blocking provably safe
non-parameterized queries (so we don't block queries that are safely built
dynamically out of static strings and variables). Many examples of this are
given in the RFC.

As you can see from this proof-of-concept (
http://phpoops.cloudapp.net/oops.php?action=maindbg_sqllimit=4%20uhoh),
all of the SQL statements are unparameterized, but only one unparameterized
SQL statement that includes attacker-controllable data is blocked, which,
as it turns out, is the only query vulnerable to SQL injection.

Dynamically construct SQL statements out of provably safe components are
not detected as a SQL-injectable error by this RFC.

== SQL injection is already solved! Why are you re-solving it? ==
This RFC doesn't solve SQL injection in a new way. It blocks dynamic SQL
queries that are not parameterized by default, but uses reverse-taint
analysis to reduce false positives where developers are building SQL query
string out of other constant strings (such as config variables, table
names, column names etc). This feature only takes action when tainted data
is concatenated into a SQL query string and that query string is issued
against the database, and even then, only if PHP.INI has been configured to
do so.

== We should log, rather than block exploit attempts ==
The feature has three modes, which is configurable via PHP.INI:
Safe mode causes the query to be aborted, and a SecurityError exception
thrown.
Logging mode causes the query to be run, and an E_WARNING raised
Ignore more causes the query to be run and no other action to be taken.

The default mode 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Joe Watkins
Morning Craig,

I think Pierre and I object to the concept, regardless of the
intricacies of any particular implementation.

Even the best implementation need only have a single hole in it and
everything falls apart, one extension not doing something properly, or one
mistake in the implementation ... it's not hard to imagine these things
happening, right ?

When I made reference to safe_mode, it wasn't because I misunderstood
the concept, I know you're not introducing another safe mode, but
introducing a INI, or build, or configuration option that says we got
this is doomed to fail whatever.

I don't like to talk about security, it's not something I feel
qualified to talk about really, but I can observe that catch-all security
features have already been proven to fail ...

It's the year 3015, we're all running PHP442, our taint implementation
is flawless, every major framework (which we have a futurish word for)
relies on tainting and thinks SQLi is no longer a thing to worry about,
because we got this. A junior sysadmin, on mars, is charged with
upgrading PHP on the server that is responsible for crop irrigation on the
terra-formed world, and forgets to configure taint properly. The upgrade
goes live, renegade ruby fans hack the system disrupting food production,
leading to the eventual death of everything.

   It's Friday afternoon, and that was a bit of fun ... I hope I lifted
your mood ... push on, I can be wrong, Pierre can be wrong, we can all be
wrong ...

Cheers
Joe

On Fri, Jul 31, 2015 at 11:40 AM, Craig Francis cr...@craigfrancis.co.uk
wrote:

 On 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote:

 Even if some of those people replying haven't read or don't understand
 what you are suggesting, it is not a good tactic to assume that and reply
 with read the RFC.



 Hi Joe,

 Sorry about yesterday, I have done as you have said, and I have read those
 responses again, but unfortunately I still feel that I was right in my
 assumptions (notes below, maybe some interesting additions?).

 In general I have been getting very frustrated that no-one seems to really
 care about security (and to be fair, it has been annoying me for far too
 many years).

 Keeping in mind that I work with other developers who routinely keep
 introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying
 things like switching the CSP off, because they copy/paste some
 hideous/insecure JS, and can't be bothered to work out why the eval
 function isn't working.

 So maybe I should start a new thread, without Matt's subject (btw Matt, I
 really appreciate what you are trying todo, I disagree with the blocking
 element, and I think we can also address more than just SQL injection
 vulnerabilities)... maybe something like I've found this one weird trick
 that will fix every security issue... sorry, I hate that kind of approach,
 but if it gets a response, maybe its worth it :-)

 Craig



 --
 http://news.php.net/php.internals/87346
 From: Matt Tait
 Reply: N/A

 Original suggestion.

 --
 http://news.php.net/php.internals/87348
 From: Rowan Collins
 Reply: Matt Tait

 Suggestion to Matt to look for previous RFCs.

 --
 http://news.php.net/php.internals/87350
 From: Christoph Becker
 Reply: Matt Tait

 Points out the existing Taint RFC from 2008, by Wietse Venema.

 --
 http://news.php.net/php.internals/87355
 From: Pierre Joye
 Reply: Matt Tait

 Pointing out that there is more than SQLi (true), and it might be an
 impossible task (I disagree, as explained later).

 --
 http://news.php.net/php.internals/87361
 From: Thomas Bley
 Reply: Matt Tait

 Suggesting the use of bound parameters / prepared statements, which I
 agree is how it should be coded by the website developers, but I think the
 PHP language itself can help identify when this has not been done (just by
 raising a notice, not blocking anything).

 Also Thomas points out that static code analysis could identify these
 issues, but these are far from perfect, and PHP is in a much better
 position to be doing this... and has the advantage of being available to
 everyone.

 Just to note, I've played with a couple of static code analysers which
 cost in the region of £19,000 per year, and they still don't find the same
 number of escaping issues that my suggestion can find (they do look at
 other issues, so don't get rid of them, but this one thing that can be done
 better with the programming language itself).

 --
 http://news.php.net/php.internals/87363
 From: Lester Caine
 Reply: Matt Tait

 Short answer saying you should not use mysql... which I think is a bit
 short sighted (on the assumption that mysqli is a similar interface for
 most 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Craig Francis
Hi Matt,

I take it all back... I've been so sure of my own suggestion, I've not really 
given your RFC a proper read... and I think this could work.

The main difference I had was the ability to support to support the escaping 
functions (see pg_escape_literal for PostgreSQL, or htmlentities for html 
output)... and your solution of not supporting any of them keeps the whole 
thing very simple.

Personally I would still like to check html encoding, which your solution won't 
do... but I realise that is quite tricky, and won't be a perfect solution:

$url = 'javascript:alert(document.cookies)';
echo 'a href=' . htmlentities($url) . '';

But may I request 2 changes:

1) Rather than calling the zend_string property sql-safe constant, could it 
simply be string literal, as in, the variable has only been created by 
T_STRING's... as this could be used for other things, e.g. executing commands 
via exec_bind:

http://news.php.net/php.internals/87361

2) Include a function that allows the PHP developers to check if the variable 
being passed is a string literal, as frameworks can check the state before 
performing an action, e.g. they could provide the exec_bind function:

function exec_bind($cmd, $arguments) {
if (!is_string_literal($cmd)) {
throw new Exception('...'); 
}
// using escapeshellarg for the $arguments
}

Craig




On 31 Jul 2015, at 14:11, Matt Tait matt.t...@gmail.com wrote:

 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications. If Google has information that indicates that SQLi is
 still more prevalent than XSS, I'd love to see this data.
 
 Thanks Scott!
 
 You are absolutely correct. SQL injection is a solved problem using
 parameterized SQL. In fact, parameterizing queries to avoid SQL injection
 is so uncontroversially accepted security advice, that this RFC *enforces* the
 advice at the language level (unless the website administrator explicitly
 disables the protection across the website via PHP.INI).
 
 Examples of SQL queries that *would* and *would not *be blocked by this
 tool are given in the RFC. In your case, ORDER BY {$column} ASC would not
 be blocked by this tool if $column came from within the PHP application,
 but would be blocked if it came from, say, $_GET[column].
 
 Sadly, although SQL injection is a solved problem by using prepared
 statements, not everyone follows the advice. Prior to working at Google, I
 worked with a company called iSEC Partners who do application review for
 various companies. From my experience, between a third and a half of all
 the often-huge PHP websites I security-reviewed were vulnerable to SQL
 injection somewhere. It wasn't uncommon for the company to have ~5000 or so
 SQL queries in their PHP code, only 2-3 of which were vulnerable to
 injection. But attackers only need one to ruin your company and steal your
 users' data.
 
 To take apart some of the other valid comments on this thread:
 
 == SQL injection is dead ==
 This is objectively not the case
 https://www.exploit-db.com/search/?action=searchdescription=SQL+injection
 
 == Won't this break every website? ==
 No. In its initial release, this feature will be released in disabled
 mode. Website owners with heightened security requirements will be able to
 upgrade this to logging or blocking mode. This means this feature will
 have zero impact on websites that do not explicitly enable it.
 
 == We should be looking at XSS instead ==
 Solving SQL injection with parameterized SQL queries is uncontroversial and
 universally accepted security advice. Lots of different (some working, some
 not) advice for solving XSS exist, but this makes it harder to standardize
 one of those ideas into the language without significant controversy.
 
 For example, one website may choose to store HTML content in their
 database. This should be printed out to HTML verbatim. Other websites may
 accidentally store HTML content in their database, leading to stored-XSS.
 The language will not be able to distinguish these two use-cases.
 
 In contrast, taking content out of a database and gluing it into a SQL
 statement is /always/ an error, as it can lead to second-order and
 blind-SQL injection vulnerabilities. For this reason, this RFC will
 initially only be looking at securing websites against SQL-injections,
 although perhaps there is scope to also look at XSS-injection
 vulnerabilities in a different RFC using similar tools at a later date.
 
 == This breaks applications that use OOP or other complicated ways of
 sending data to a database ==
 This is not the case. This RFC doesn't use taint analysis to find SQL
 injections; it uses reverse-taint analysis to avoid blocking provably safe
 non-parameterized queries (so we don't block 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Lester Caine
On 28/07/15 18:33, Matt Tait wrote:
 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.

So who addresses all the other database drivers? Which is something
other ''proposals' currently ignore as well.

-- 
Lester Caine - G8HFL
-
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Ronald Chmara
On Thu, Jul 30, 2015 at 8:38 AM, Craig Francis cr...@craigfrancis.co.uk
wrote:

 On 30 Jul 2015, at 16:26, Ronald Chmara rona...@gmail.com wrote:
  Perhaps I have missed something in this discussion
 I think you have... my email from a couple of weeks ago was ignored... so
 I replied to Matt's suggestion (which is similar, but different).
 Please, just spend a few minutes reading my suggestion, it has absolutely
 nothing todo with breaking applications:
http://news.php.net/php.internals/87207

   https://bugs.php.net/bug.php?id=69886


The RFC and bug report both make an erroneous assumption that unescaped GPC
input is wrong.

I was addressing some cases where it is the correct, intended, behavior.

WRT breaking:
Application stacks which go from zero E_NOTICE warnings, to hundreds or
thousands of them a second or day, is (admittedly) poorly phrased as
breaking. It is a possible side effect of the proposed solutiions. I have
worked in very stingent environments where an unapproved E_NOTICE is
considered a ship-stop break, but I did not explicitly explain that. Such
environments would require re-writes of all SQL that throws an E_NOTICE, or
a per-line exception review and approval process, or disabling/not enabling
the checking.


 And yes, I do have a bypass_the_nerfing function (well, a function to say
 the variable has already been escaped)... but the idea is that it's ever so
 slightly harder to use than the related escaping functions, and rarely
 needed.


Rarely is subjective at this point, a quanyifyable sampling of some kind
could be more meaningful. (How rare?)

Based on *my* purely anecdotal experience, in the last X years of using PHP
I have have frequently encountered situations where passing through
engine-unescaped text strings, to SQL, is desired for some reason, in
nearly every environment. I mentioned one use case that I thought might be
relevant to a large number of users, there are others.

Off the top of my head, some use cases I have dealt with relevant to this
discussion, that should be considered (even if they're discarded as
trivial):
1. Administrator has a web application that is supposed to allow them
access functionally equivalent to a direct connection to a database.
2. Overhead of using the engine escaping technique (setup connection(if not
done yet), send text to escape at network speed, recieve escaped text at
network speed) was considered too much of an additional performance hit.
3. Text needed to have a generic, user written, escaping library/function
to handle multiple destinations (think 5 different data storage systems,
all with different escaping rules, some without connection based escaping).
4. Text being supplied was coming from a pre-cleaned, trusted, source, even
though the variable was GPC, (example: it was a GET string assembled by a
batch job that was pulling from another database)

I'm sure there are many more.

Basing language decisions on personal perceptions of rarely and
frequently is not a good practice, but ensuring that we are covering
multiple use-cases is. Discussing the use cases doesn't mean I think the
idea is without merit.

-Ronabop


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-31 Thread Craig Francis
On 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote:

 Even if some of those people replying haven't read or don't understand what 
 you are suggesting, it is not a good tactic to assume that and reply with 
 read the RFC.



Hi Joe,

Sorry about yesterday, I have done as you have said, and I have read those 
responses again, but unfortunately I still feel that I was right in my 
assumptions (notes below, maybe some interesting additions?).

In general I have been getting very frustrated that no-one seems to really care 
about security (and to be fair, it has been annoying me for far too many years).

Keeping in mind that I work with other developers who routinely keep 
introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying things 
like switching the CSP off, because they copy/paste some hideous/insecure JS, 
and can't be bothered to work out why the eval function isn't working.

So maybe I should start a new thread, without Matt's subject (btw Matt, I 
really appreciate what you are trying todo, I disagree with the blocking 
element, and I think we can also address more than just SQL injection 
vulnerabilities)... maybe something like I've found this one weird trick that 
will fix every security issue... sorry, I hate that kind of approach, but if 
it gets a response, maybe its worth it :-)

Craig



--
http://news.php.net/php.internals/87346
From: Matt Tait
Reply: N/A

Original suggestion.

--
http://news.php.net/php.internals/87348
From: Rowan Collins
Reply: Matt Tait

Suggestion to Matt to look for previous RFCs.

--
http://news.php.net/php.internals/87350
From: Christoph Becker
Reply: Matt Tait

Points out the existing Taint RFC from 2008, by Wietse Venema.

--
http://news.php.net/php.internals/87355
From: Pierre Joye
Reply: Matt Tait

Pointing out that there is more than SQLi (true), and it might be an impossible 
task (I disagree, as explained later).

--
http://news.php.net/php.internals/87361
From: Thomas Bley
Reply: Matt Tait

Suggesting the use of bound parameters / prepared statements, which I agree is 
how it should be coded by the website developers, but I think the PHP language 
itself can help identify when this has not been done (just by raising a notice, 
not blocking anything).

Also Thomas points out that static code analysis could identify these issues, 
but these are far from perfect, and PHP is in a much better position to be 
doing this... and has the advantage of being available to everyone.

Just to note, I've played with a couple of static code analysers which cost in 
the region of £19,000 per year, and they still don't find the same number of 
escaping issues that my suggestion can find (they do look at other issues, so 
don't get rid of them, but this one thing that can be done better with the 
programming language itself).

--
http://news.php.net/php.internals/87363
From: Lester Caine
Reply: Matt Tait

Short answer saying you should not use mysql... which I think is a bit short 
sighted (on the assumption that mysqli is a similar interface for most website 
developers).

I'm not against using tools like ORMs (e.g. Doctrine), bound parameters / 
prepared statements, or even stored procedures... but they all have issues, and 
are all vulnerable to the kind of misuse I'm trying to address (explained in 
the next email).

--
http://news.php.net/php.internals/87370
From: Me (Craig Francis)
Reply: Lester Caine

Just saying I disagree with Lester, and giving a very simple example of how 
developers can still mess up (once you start adding some abstractions, like an 
ORM, this becomes much harder to detect, and is why I'm so insistent that PHP 
needs to be checking for these issues).

This is where I also suggest an alternative to Matt's original suggestion 
(something I posted 12 days before, and didn't really get any response).

--
http://news.php.net/php.internals/87383
From: Lester Caine
Reply: Me (Craig Francis)

Kind of missing the point (maybe my example was too simple), and is talking 
about how Matts solution would cause problems.

And I agree (sorry Matt), I don't think Matt's solution would work... but if 
Lester had read my reply, he would have seen that my suggestion was about 
education (but it can also help experienced developers as well).

--
http://news.php.net/php.internals/87386
From: Me (Craig Francis)
Reply: Lester Caine

Trying to explain to Lester that I agree on education, and pointing out that my 
solution is different... and how (maybe my email was too long to read?).


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Craig Francis
On 30 Jul 2015, at 13:14, Joe Watkins pthre...@pthreads.org wrote:

 I find myself agreeing with Pierre; The wrong signal would be sent. History 
 should teach us there is no such thing as (a) safe mode.



Hi Joe,

Please can you read my proposal (see the email you just replied to, also 
below)... I'm replying on this thread because my first one was ignored... I'm 
not suggesting a safe mode or any kind of blocking of requests (as per the 
subject)... as I agree, and believe that would be worse than the old auto 
escaping from PHP 4.

Craig






On 30 Jul 2015, at 13:14, Joe Watkins pthre...@pthreads.org wrote:

 I find myself agreeing with Pierre; The wrong signal would be sent. History 
 should teach us there is no such thing as (a) safe mode.
 
 Xinchen did used to work on a taint extension, I wonder why that was stopped 
 ? 
 
 Worth noticing that the extension is rather complex, touching many parts of 
 the engine, changing many things ... which I don't really like.
 
 Cheers
 Joe
 
 On Thu, Jul 30, 2015 at 10:14 AM, Craig Francis cr...@craigfrancis.co.uk 
 wrote:
 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:
 
  But that is a perfect example of what I am talking about. You do not
  educate people by publishing the very thing that is wrong. You educate
  them by pointing out to them WHY the '?' was there in the first place.
 
 
 
 
 I completely agree on education, and what I'm hoping for... and this is how 
 we can educate everyone :-)
 
 My suggestion for taints (not quite the same as the one from Matt or Wietse) 
 was not to change the way good programs are created/executed, but simply an 
 education device, which can also pick up mistakes that experienced developers 
 make.
 
 While my first post on this mailing list gives a better overview:
 
http://news.php.net/php.internals/87207
 
 The original implementation suggestion is at:
 
https://bugs.php.net/bug.php?id=69886
 
 You will see that it does nothing more than create notices to say erm, do 
 you want to be doing this?.
 
 This is something that only PHP can do, unless you can find a way of changing 
 every single article / code example on the internet :-)
 
 So, with your example... if you want to use a variable for a table/field 
 prefix, that is perfectly fine... in fact, it won't need any changes, as the 
 prefix will probably be hard coded as a string within a PHP script (something 
 I called ETYPE_CONSTANT).
 
 But if not (e.g. storing the prefix in an ini file?), then I've shown an 
 example of how that can be handled with the proposed string_encoding_set 
 function (something I should have probably called string_escaping_set)... 
 which is simply to tell PHP that this one variable is already safe (something 
 I can't see being needed very often).
 
 Craig
 
 
 
 
 
 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:
 
  On 29/07/15 16:11, Craig Francis wrote:
  I completely disagree... prepared statements are just as vulnerable, and 
  so are ORM's.
 
  You can push developers towards these solutions, and that would be good, 
  but you are completely blind if you think an uneducated developer won't do:
 
   if ($stmt = $mysqli-prepare(SELECT District FROM City WHERE Name= 
  . $_GET['name'])) {
   }
 
  But that is a perfect example of what I am talking about. You do not
  educate people by publishing the very thing that is wrong. You educate
  them by pointing out to them WHY the '?' was there in the first place.
 
  Since the taint extension only covers mysql and sqlite it's of little
  use if we manage to convert 'uneducated developer' to any of the more
  secure databases, and that was one of the reasons why mysql was dropped
  from being loaded by default. Once one starts from a base of
  parametrised sql queries the lax programming methods many mysql guides
  and books continue to push can be reversed. Throwing more bloat into php
  to create 'WTF' errors just adds to a new users frustration and annoys
  experienced users who have very good reasons for building queries using
  clean variables. MANY abstraction layers use variables to add prefixes
  to table names or fields.
 
  Educate ... don't nanny ...
 
  --
  Lester Caine - G8HFL
  -
  Contact - http://lsces.co.uk/wiki/?page=contact
  L.S.Caine Electronic Services - http://lsces.co.uk
  EnquirySolve - http://enquirysolve.com/
  Model Engineers Digital Workshop - http://medw.co.uk
  Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
 
  --
  PHP Internals - PHP Runtime Development Mailing List
  To unsubscribe, visit: http://www.php.net/unsub.php
 
 
 
 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php
 
 



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Joe Watkins
I find myself agreeing with Pierre; The wrong signal would be sent. History
should teach us there is no such thing as (a) safe mode.

Xinchen did used to work on a taint extension, I wonder why that was
stopped ?

Worth noticing that the extension is rather complex, touching many parts of
the engine, changing many things ... which I don't really like.

Cheers
Joe

On Thu, Jul 30, 2015 at 10:14 AM, Craig Francis cr...@craigfrancis.co.uk
wrote:

 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:

  But that is a perfect example of what I am talking about. You do not
  educate people by publishing the very thing that is wrong. You educate
  them by pointing out to them WHY the '?' was there in the first place.




 I completely agree on education, and what I'm hoping for... and this is
 how we can educate everyone :-)

 My suggestion for taints (not quite the same as the one from Matt or
 Wietse) was not to change the way good programs are created/executed, but
 simply an education device, which can also pick up mistakes that
 experienced developers make.

 While my first post on this mailing list gives a better overview:

http://news.php.net/php.internals/87207

 The original implementation suggestion is at:

https://bugs.php.net/bug.php?id=69886

 You will see that it does nothing more than create notices to say erm, do
 you want to be doing this?.

 This is something that only PHP can do, unless you can find a way of
 changing every single article / code example on the internet :-)

 So, with your example... if you want to use a variable for a table/field
 prefix, that is perfectly fine... in fact, it won't need any changes, as
 the prefix will probably be hard coded as a string within a PHP script
 (something I called ETYPE_CONSTANT).

 But if not (e.g. storing the prefix in an ini file?), then I've shown an
 example of how that can be handled with the proposed string_encoding_set
 function (something I should have probably called string_escaping_set)...
 which is simply to tell PHP that this one variable is already safe
 (something I can't see being needed very often).

 Craig





 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:

  On 29/07/15 16:11, Craig Francis wrote:
  I completely disagree... prepared statements are just as vulnerable,
 and so are ORM's.
 
  You can push developers towards these solutions, and that would be
 good, but you are completely blind if you think an uneducated developer
 won't do:
 
   if ($stmt = $mysqli-prepare(SELECT District FROM City WHERE
 Name= . $_GET['name'])) {
   }
 
  But that is a perfect example of what I am talking about. You do not
  educate people by publishing the very thing that is wrong. You educate
  them by pointing out to them WHY the '?' was there in the first place.
 
  Since the taint extension only covers mysql and sqlite it's of little
  use if we manage to convert 'uneducated developer' to any of the more
  secure databases, and that was one of the reasons why mysql was dropped
  from being loaded by default. Once one starts from a base of
  parametrised sql queries the lax programming methods many mysql guides
  and books continue to push can be reversed. Throwing more bloat into php
  to create 'WTF' errors just adds to a new users frustration and annoys
  experienced users who have very good reasons for building queries using
  clean variables. MANY abstraction layers use variables to add prefixes
  to table names or fields.
 
  Educate ... don't nanny ...
 
  --
  Lester Caine - G8HFL
  -
  Contact - http://lsces.co.uk/wiki/?page=contact
  L.S.Caine Electronic Services - http://lsces.co.uk
  EnquirySolve - http://enquirysolve.com/
  Model Engineers Digital Workshop - http://medw.co.uk
  Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
 
  --
  PHP Internals - PHP Runtime Development Mailing List
  To unsubscribe, visit: http://www.php.net/unsub.php
 


 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php




Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Xinchen Hui
Hey:

On Thu, Jul 30, 2015 at 8:14 PM, Joe Watkins pthre...@pthreads.org wrote:
 I find myself agreeing with Pierre; The wrong signal would be sent. History
 should teach us there is no such thing as (a) safe mode.

 Xinchen did used to work on a taint extension, I wonder why that was
 stopped ?
yes, it is https://github.com/laruence/php-taint

Anyway, I was too busy so I didn't make it supports PHP-5.6, I was
hoping someone could help(it supports 5.5 now).

it is a complex extension, and using tricky way to keep taint infos

anyway, with PHP7's new zend_string, and string flags, the
implementation will become easier.

I have a plan to make it supports PHP7..

thanks

 Worth noticing that the extension is rather complex, touching many parts of
 the engine, changing many things ... which I don't really like.

 Cheers
 Joe

 On Thu, Jul 30, 2015 at 10:14 AM, Craig Francis cr...@craigfrancis.co.uk
 wrote:

 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:

  But that is a perfect example of what I am talking about. You do not
  educate people by publishing the very thing that is wrong. You educate
  them by pointing out to them WHY the '?' was there in the first place.




 I completely agree on education, and what I'm hoping for... and this is
 how we can educate everyone :-)

 My suggestion for taints (not quite the same as the one from Matt or
 Wietse) was not to change the way good programs are created/executed, but
 simply an education device, which can also pick up mistakes that
 experienced developers make.

 While my first post on this mailing list gives a better overview:

http://news.php.net/php.internals/87207

 The original implementation suggestion is at:

https://bugs.php.net/bug.php?id=69886

 You will see that it does nothing more than create notices to say erm, do
 you want to be doing this?.

 This is something that only PHP can do, unless you can find a way of
 changing every single article / code example on the internet :-)

 So, with your example... if you want to use a variable for a table/field
 prefix, that is perfectly fine... in fact, it won't need any changes, as
 the prefix will probably be hard coded as a string within a PHP script
 (something I called ETYPE_CONSTANT).

 But if not (e.g. storing the prefix in an ini file?), then I've shown an
 example of how that can be handled with the proposed string_encoding_set
 function (something I should have probably called string_escaping_set)...
 which is simply to tell PHP that this one variable is already safe
 (something I can't see being needed very often).

 Craig





 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:

  On 29/07/15 16:11, Craig Francis wrote:
  I completely disagree... prepared statements are just as vulnerable,
 and so are ORM's.
 
  You can push developers towards these solutions, and that would be
 good, but you are completely blind if you think an uneducated developer
 won't do:
 
   if ($stmt = $mysqli-prepare(SELECT District FROM City WHERE
 Name= . $_GET['name'])) {
   }
 
  But that is a perfect example of what I am talking about. You do not
  educate people by publishing the very thing that is wrong. You educate
  them by pointing out to them WHY the '?' was there in the first place.
 
  Since the taint extension only covers mysql and sqlite it's of little
  use if we manage to convert 'uneducated developer' to any of the more
  secure databases, and that was one of the reasons why mysql was dropped
  from being loaded by default. Once one starts from a base of
  parametrised sql queries the lax programming methods many mysql guides
  and books continue to push can be reversed. Throwing more bloat into php
  to create 'WTF' errors just adds to a new users frustration and annoys
  experienced users who have very good reasons for building queries using
  clean variables. MANY abstraction layers use variables to add prefixes
  to table names or fields.
 
  Educate ... don't nanny ...
 
  --
  Lester Caine - G8HFL
  -
  Contact - http://lsces.co.uk/wiki/?page=contact
  L.S.Caine Electronic Services - http://lsces.co.uk
  EnquirySolve - http://enquirysolve.com/
  Model Engineers Digital Workshop - http://medw.co.uk
  Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
 
  --
  PHP Internals - PHP Runtime Development Mailing List
  To unsubscribe, visit: http://www.php.net/unsub.php
 


 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php





-- 
Xinchen Hui
@Laruence
http://www.laruence.com/

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Craig Francis
On 30 Jul 2015, at 16:24, Scott Arciszewski sc...@paragonie.com wrote:

  Just because the solution is known doesn't mean it's known to everyone.



Yes, and if you could just read what I was suggesting, rather than looking at 
the subject of this email (and the suggestion by Matt), then you will notice 
this is what I'm trying to do (so not just people asking questions on Stack 
Overflow).

My suggestion is to educate, it also has a nice side effect of having a simple 
checking process for everything else (without breaking anything).









On 30 Jul 2015, at 16:24, Scott Arciszewski sc...@paragonie.com wrote:

 On Thu, Jul 30, 2015 at 11:20 AM, Craig Francis
 cr...@craigfrancis.co.uk wrote:
 On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com wrote:
 
 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications.
 
 
 Good, because my suggestion was also addressing XSS with poor (or completely 
 missing) HTML escaping... have a look:
 
  http://news.php.net/php.internals/87207
 
  https://bugs.php.net/bug.php?id=69886
 
 Now I admit it won't fix everything with XSS (as HTML escaping is a bit 
 harder), but it certainly will pick up quite a lot of the issues (and it 
 wont break anything either, just help developers identify problems).
 
 And no, SQL injection is far from a solved problem... this is why, after 15 
 years of me trying to tell my fellow developers to not make these mistakes, 
 I'm still finding them making them over and over again... hence why I'm 
 making the above suggestion.
 
 Craig
 
 
 
 
 
 
 On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com wrote:
 
 On Tue, Jul 28, 2015 at 1:33 PM, Matt Tait matt.t...@gmail.com wrote:
 Hi all,
 
 I've written an RFC (and PoC) about automatic detection and blocking of SQL
 injection vulnerabilities directly from inside PHP via automated taint
 analysis.
 
 https://wiki.php.net/rfc/sql_injection_protection
 
 In short, we make zend_strings track where their value originated. If it
 originated as a T_STRING, from a primitive (like int) promotion, or as a
 concatenation of such strings, it's query that can't have been SQL-injected
 by an attacker controlled string. If we can't prove that the query is safe,
 that means that the query is either certainly vulnerable to a SQL-injection
 vulnerability, or sufficiently complex that it should be parameterized
 just-to-be-sure.
 
 There's also a working proof of concept over here:
 
 http://phpoops.cloudapp.net/oops.php
 
 You'll notice that the page makes a large number of SQL statements, most of
 which are not vulnerable to SQL injection, but one is. The proof of concept
 is smart enough to block that one vulnerable request, and leave all of the
 others unchanged.
 
 In terms of performance, the cost here is negligible. This is just basic
 variable taint analysis under the hood, (not an up-front intraprocedurale
 static analysis or anything complex) so there's basically no slow down.
 
 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.
 
 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.
 
 Matt
 
 Hi Matt,
 
 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.
 
 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications. If Google has information that indicates that SQLi is
 still more prevalent than XSS, I'd love to see this data.
 
 In my opinion, SQL injection is almost a solved problem. Use prepared
 statements where you can, and strictly whitelist where you cannot
 (i.e. ORDER BY {$column} ASC)
 
 Scott Arciszewski
 Chief Development Officer
 Paragon Initiative Enterprises https://paragonie.com
 
 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php
 
 
 
 Just because the solution is known doesn't mean it's known to
 everyone. Diffusion of knowledge and good habits is the hardest
 problem in application security to solve. Look, for example, at how
 many college students learn to write 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Scott Arciszewski
On Thu, Jul 30, 2015 at 11:20 AM, Craig Francis
cr...@craigfrancis.co.uk wrote:
 On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com wrote:

 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications.


 Good, because my suggestion was also addressing XSS with poor (or completely 
 missing) HTML escaping... have a look:

   http://news.php.net/php.internals/87207

   https://bugs.php.net/bug.php?id=69886

 Now I admit it won't fix everything with XSS (as HTML escaping is a bit 
 harder), but it certainly will pick up quite a lot of the issues (and it wont 
 break anything either, just help developers identify problems).

 And no, SQL injection is far from a solved problem... this is why, after 15 
 years of me trying to tell my fellow developers to not make these mistakes, 
 I'm still finding them making them over and over again... hence why I'm 
 making the above suggestion.

 Craig






 On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com wrote:

 On Tue, Jul 28, 2015 at 1:33 PM, Matt Tait matt.t...@gmail.com wrote:
 Hi all,

 I've written an RFC (and PoC) about automatic detection and blocking of SQL
 injection vulnerabilities directly from inside PHP via automated taint
 analysis.

 https://wiki.php.net/rfc/sql_injection_protection

 In short, we make zend_strings track where their value originated. If it
 originated as a T_STRING, from a primitive (like int) promotion, or as a
 concatenation of such strings, it's query that can't have been SQL-injected
 by an attacker controlled string. If we can't prove that the query is safe,
 that means that the query is either certainly vulnerable to a SQL-injection
 vulnerability, or sufficiently complex that it should be parameterized
 just-to-be-sure.

 There's also a working proof of concept over here:

 http://phpoops.cloudapp.net/oops.php

 You'll notice that the page makes a large number of SQL statements, most of
 which are not vulnerable to SQL injection, but one is. The proof of concept
 is smart enough to block that one vulnerable request, and leave all of the
 others unchanged.

 In terms of performance, the cost here is negligible. This is just basic
 variable taint analysis under the hood, (not an up-front intraprocedurale
 static analysis or anything complex) so there's basically no slow down.

 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.

 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.

 Matt

 Hi Matt,

 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.

 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications. If Google has information that indicates that SQLi is
 still more prevalent than XSS, I'd love to see this data.

 In my opinion, SQL injection is almost a solved problem. Use prepared
 statements where you can, and strictly whitelist where you cannot
 (i.e. ORDER BY {$column} ASC)

 Scott Arciszewski
 Chief Development Officer
 Paragon Initiative Enterprises https://paragonie.com

 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php



Just because the solution is known doesn't mean it's known to
everyone. Diffusion of knowledge and good habits is the hardest
problem in application security to solve. Look, for example, at how
many college students learn to write C programs with buffer overflow
vulnerabilities in 2015. We need more effort on education, which is
part of what I've been focusing on with Paragon Initiative and Stack
Overflow.

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises https://paragonie.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Craig Francis
On 30 Jul 2015, at 16:26, Ronald Chmara rona...@gmail.com wrote:

 Perhaps I have missed something in this discussion


I think you have... my email from a couple of weeks ago was ignored... so I 
replied to Matt's suggestion (which is similar, but different).

Please, just spend a few minutes reading my suggestion, it has absolutely 
nothing todo with breaking applications:

   http://news.php.net/php.internals/87207

   https://bugs.php.net/bug.php?id=69886

And yes, I do have a bypass_the_nerfing function (well, a function to say the 
variable has already been escaped)... but the idea is that it's ever so 
slightly harder to use than the related escaping functions, and rarely needed.






On 30 Jul 2015, at 16:26, Ronald Chmara rona...@gmail.com wrote:

 Perhaps I have missed something in this discussion where such a change to PHP 
 does not break every single application that is supposed to pass raw, user 
 submitted, SQL *without* getting prepared/nerfed, or warned about, by 
 intentional application design.
 
 If we're just limiting the nerfing for submitted GPC variables (since PHP is 
 used a lot for web applications) we still have a non-trivial number of 
 those installed applications which require raw, user created, unescaped SQL, 
 passing through to function as designed.
 
 I am thinking of the class of applications like phpMyAdmin, as well as the 
 the millions of other database utility scripts, application install scripts, 
 (etc.) out there that perform similar tasks, that need to pass raw SQL, as 
 crafted by users, without preparation, intentionally.
 
 Of course, we could just add a bypass_the_nerfing() function, and such a 
 function could then possibly see widespread adoption, everywhere, rendering 
 the entire exercise moot.


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Craig Francis
On 30 Jul 2015, at 13:47, Xinchen Hui larue...@php.net wrote:
 anyway, with PHP7's new zend_string, and string flags, the
 implementation will become easier.



Hi Xinchen,

Glad to hear that you are still looking into this... please let me know if 
there is anything I can do to help (unfortunately I'm not a C programer).

Out of interest, if you are going to continue using taint_marks, as the RFC 
suggested... can I suggest all variables start with 0 (or undefined)... then 
you can set flags as the variables are passed though functions like 
htmlentities and pg_escape_literal?

This way all variables are treated as plain (unsafe), and then developers 
either need to escape them (e.g. when printing to output, or using in SQL, CLI, 
etc), or they can mark them as having already been escaped (rare).

Likewise, I know you have examples that say SCRIPT_FILENAME is safe by 
default (I kind of disagree)... it would still be advisable to encode them, 
even if they are being included in the HTML... personally I would not have any 
variables marked as safe by default (with the single exception of strings that 
are defined in the PHP code itself).

Craig






On 30 Jul 2015, at 13:47, Xinchen Hui larue...@php.net wrote:

 Hey:
 
 On Thu, Jul 30, 2015 at 8:14 PM, Joe Watkins pthre...@pthreads.org wrote:
 I find myself agreeing with Pierre; The wrong signal would be sent. History
 should teach us there is no such thing as (a) safe mode.
 
 Xinchen did used to work on a taint extension, I wonder why that was
 stopped ?
 yes, it is https://github.com/laruence/php-taint
 
 Anyway, I was too busy so I didn't make it supports PHP-5.6, I was
 hoping someone could help(it supports 5.5 now).
 
 it is a complex extension, and using tricky way to keep taint infos
 
 anyway, with PHP7's new zend_string, and string flags, the
 implementation will become easier.
 
 I have a plan to make it supports PHP7..
 
 thanks
 
 Worth noticing that the extension is rather complex, touching many parts of
 the engine, changing many things ... which I don't really like.
 
 Cheers
 Joe
 
 On Thu, Jul 30, 2015 at 10:14 AM, Craig Francis cr...@craigfrancis.co.uk
 wrote:
 
 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:
 
 But that is a perfect example of what I am talking about. You do not
 educate people by publishing the very thing that is wrong. You educate
 them by pointing out to them WHY the '?' was there in the first place.
 
 
 
 
 I completely agree on education, and what I'm hoping for... and this is
 how we can educate everyone :-)
 
 My suggestion for taints (not quite the same as the one from Matt or
 Wietse) was not to change the way good programs are created/executed, but
 simply an education device, which can also pick up mistakes that
 experienced developers make.
 
 While my first post on this mailing list gives a better overview:
 
   http://news.php.net/php.internals/87207
 
 The original implementation suggestion is at:
 
   https://bugs.php.net/bug.php?id=69886
 
 You will see that it does nothing more than create notices to say erm, do
 you want to be doing this?.
 
 This is something that only PHP can do, unless you can find a way of
 changing every single article / code example on the internet :-)
 
 So, with your example... if you want to use a variable for a table/field
 prefix, that is perfectly fine... in fact, it won't need any changes, as
 the prefix will probably be hard coded as a string within a PHP script
 (something I called ETYPE_CONSTANT).
 
 But if not (e.g. storing the prefix in an ini file?), then I've shown an
 example of how that can be handled with the proposed string_encoding_set
 function (something I should have probably called string_escaping_set)...
 which is simply to tell PHP that this one variable is already safe
 (something I can't see being needed very often).
 
 Craig
 
 
 
 
 
 On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:
 
 On 29/07/15 16:11, Craig Francis wrote:
 I completely disagree... prepared statements are just as vulnerable,
 and so are ORM's.
 
 You can push developers towards these solutions, and that would be
 good, but you are completely blind if you think an uneducated developer
 won't do:
 
 if ($stmt = $mysqli-prepare(SELECT District FROM City WHERE
 Name= . $_GET['name'])) {
 }
 
 But that is a perfect example of what I am talking about. You do not
 educate people by publishing the very thing that is wrong. You educate
 them by pointing out to them WHY the '?' was there in the first place.
 
 Since the taint extension only covers mysql and sqlite it's of little
 use if we manage to convert 'uneducated developer' to any of the more
 secure databases, and that was one of the reasons why mysql was dropped
 from being loaded by default. Once one starts from a base of
 parametrised sql queries the lax programming methods many mysql guides
 and books continue to push can be reversed. Throwing more bloat into php
 to create 'WTF' errors 

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Craig Francis
On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com wrote:

 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications.


Good, because my suggestion was also addressing XSS with poor (or completely 
missing) HTML escaping... have a look:

  http://news.php.net/php.internals/87207

  https://bugs.php.net/bug.php?id=69886

Now I admit it won't fix everything with XSS (as HTML escaping is a bit 
harder), but it certainly will pick up quite a lot of the issues (and it wont 
break anything either, just help developers identify problems).

And no, SQL injection is far from a solved problem... this is why, after 15 
years of me trying to tell my fellow developers to not make these mistakes, I'm 
still finding them making them over and over again... hence why I'm making the 
above suggestion.

Craig






On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com wrote:

 On Tue, Jul 28, 2015 at 1:33 PM, Matt Tait matt.t...@gmail.com wrote:
 Hi all,
 
 I've written an RFC (and PoC) about automatic detection and blocking of SQL
 injection vulnerabilities directly from inside PHP via automated taint
 analysis.
 
 https://wiki.php.net/rfc/sql_injection_protection
 
 In short, we make zend_strings track where their value originated. If it
 originated as a T_STRING, from a primitive (like int) promotion, or as a
 concatenation of such strings, it's query that can't have been SQL-injected
 by an attacker controlled string. If we can't prove that the query is safe,
 that means that the query is either certainly vulnerable to a SQL-injection
 vulnerability, or sufficiently complex that it should be parameterized
 just-to-be-sure.
 
 There's also a working proof of concept over here:
 
 http://phpoops.cloudapp.net/oops.php
 
 You'll notice that the page makes a large number of SQL statements, most of
 which are not vulnerable to SQL injection, but one is. The proof of concept
 is smart enough to block that one vulnerable request, and leave all of the
 others unchanged.
 
 In terms of performance, the cost here is negligible. This is just basic
 variable taint analysis under the hood, (not an up-front intraprocedurale
 static analysis or anything complex) so there's basically no slow down.
 
 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.
 
 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.
 
 Matt
 
 Hi Matt,
 
 PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
 injections are the result of a developer either not understanding how to
 prevent SQL injection, or taking a shortcut because it's fewer keystrokes
 to do it a feels safe rather than is safe way.
 
 This may have been true at one point in time, but my own experience
 and the statistics collected by Dan Kaminsky of White Hat Security
 indicates that Cross-Site Scripting vulnerabilities are much more
 prevalent in 2015 than SQL Injection, especially in business
 applications. If Google has information that indicates that SQLi is
 still more prevalent than XSS, I'd love to see this data.
 
 In my opinion, SQL injection is almost a solved problem. Use prepared
 statements where you can, and strictly whitelist where you cannot
 (i.e. ORDER BY {$column} ASC)
 
 Scott Arciszewski
 Chief Development Officer
 Paragon Initiative Enterprises https://paragonie.com
 
 -- 
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php
 


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Ronald Chmara
Perhaps I have missed something in this discussion where such a change to
PHP does not break every single application that is supposed to pass raw,
user submitted, SQL *without* getting prepared/nerfed, or warned about, by
intentional application design.

If we're just limiting the nerfing for submitted GPC variables (since PHP
is used a lot for web applications) we still have a non-trivial number
of those installed applications which require raw, user created, unescaped
SQL, passing through to function as designed.

I am thinking of the class of applications like phpMyAdmin, as well as the
the millions of other database utility scripts, application install
scripts, (etc.) out there that perform similar tasks, that need to pass raw
SQL, as crafted by users, without preparation, intentionally.

Of course, we could just add a bypass_the_nerfing() function, and such a
function could then possibly see widespread adoption, everywhere, rendering
the entire exercise moot.


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Lester Caine
On 29/07/15 16:11, Craig Francis wrote:
 I completely disagree... prepared statements are just as vulnerable, and so 
 are ORM's.
 
 You can push developers towards these solutions, and that would be good, but 
 you are completely blind if you think an uneducated developer won't do:
 
   if ($stmt = $mysqli-prepare(SELECT District FROM City WHERE Name= . 
 $_GET['name'])) {
   }

But that is a perfect example of what I am talking about. You do not
educate people by publishing the very thing that is wrong. You educate
them by pointing out to them WHY the '?' was there in the first place.

Since the taint extension only covers mysql and sqlite it's of little
use if we manage to convert 'uneducated developer' to any of the more
secure databases, and that was one of the reasons why mysql was dropped
from being loaded by default. Once one starts from a base of
parametrised sql queries the lax programming methods many mysql guides
and books continue to push can be reversed. Throwing more bloat into php
to create 'WTF' errors just adds to a new users frustration and annoys
experienced users who have very good reasons for building queries using
clean variables. MANY abstraction layers use variables to add prefixes
to table names or fields.

Educate ... don't nanny ...

-- 
Lester Caine - G8HFL
-
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Joe Watkins
Even if some of those people replying haven't read or don't understand what
you are suggesting, it is not a good tactic to assume that and reply with
read the RFC.

There is a good chance the majority of the people replying have read the
RFC, and found reason to be negative, reserved, cautious, or whatever.

The best thing you can do now is read those responses again, and try to
find what they are saying, if you want the conversation to continue.

Cheers
Joe

On Thu, Jul 30, 2015 at 4:38 PM, Craig Francis cr...@craigfrancis.co.uk
wrote:

 On 30 Jul 2015, at 16:24, Scott Arciszewski sc...@paragonie.com wrote:

   Just because the solution is known doesn't mean it's known to everyone.



 Yes, and if you could just read what I was suggesting, rather than looking
 at the subject of this email (and the suggestion by Matt), then you will
 notice this is what I'm trying to do (so not just people asking questions
 on Stack Overflow).

 My suggestion is to educate, it also has a nice side effect of having a
 simple checking process for everything else (without breaking anything).









 On 30 Jul 2015, at 16:24, Scott Arciszewski sc...@paragonie.com wrote:

  On Thu, Jul 30, 2015 at 11:20 AM, Craig Francis
  cr...@craigfrancis.co.uk wrote:
  On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com
 wrote:
 
  This may have been true at one point in time, but my own experience
  and the statistics collected by Dan Kaminsky of White Hat Security
  indicates that Cross-Site Scripting vulnerabilities are much more
  prevalent in 2015 than SQL Injection, especially in business
  applications.
 
 
  Good, because my suggestion was also addressing XSS with poor (or
 completely missing) HTML escaping... have a look:
 
   http://news.php.net/php.internals/87207
 
   https://bugs.php.net/bug.php?id=69886
 
  Now I admit it won't fix everything with XSS (as HTML escaping is a bit
 harder), but it certainly will pick up quite a lot of the issues (and it
 wont break anything either, just help developers identify problems).
 
  And no, SQL injection is far from a solved problem... this is why,
 after 15 years of me trying to tell my fellow developers to not make these
 mistakes, I'm still finding them making them over and over again... hence
 why I'm making the above suggestion.
 
  Craig
 
 
 
 
 
 
  On 30 Jul 2015, at 14:43, Scott Arciszewski sc...@paragonie.com
 wrote:
 
  On Tue, Jul 28, 2015 at 1:33 PM, Matt Tait matt.t...@gmail.com
 wrote:
  Hi all,
 
  I've written an RFC (and PoC) about automatic detection and blocking
 of SQL
  injection vulnerabilities directly from inside PHP via automated taint
  analysis.
 
  https://wiki.php.net/rfc/sql_injection_protection
 
  In short, we make zend_strings track where their value originated. If
 it
  originated as a T_STRING, from a primitive (like int) promotion, or
 as a
  concatenation of such strings, it's query that can't have been
 SQL-injected
  by an attacker controlled string. If we can't prove that the query is
 safe,
  that means that the query is either certainly vulnerable to a
 SQL-injection
  vulnerability, or sufficiently complex that it should be parameterized
  just-to-be-sure.
 
  There's also a working proof of concept over here:
 
  http://phpoops.cloudapp.net/oops.php
 
  You'll notice that the page makes a large number of SQL statements,
 most of
  which are not vulnerable to SQL injection, but one is. The proof of
 concept
  is smart enough to block that one vulnerable request, and leave all
 of the
  others unchanged.
 
  In terms of performance, the cost here is negligible. This is just
 basic
  variable taint analysis under the hood, (not an up-front
 intraprocedurale
  static analysis or anything complex) so there's basically no slow
 down.
 
  PHP SQL injections are the #1 way PHP applications get hacked - and
 all SQL
  injections are the result of a developer either not understanding how
 to
  prevent SQL injection, or taking a shortcut because it's fewer
 keystrokes
  to do it a feels safe rather than is safe way.
 
  What do you all think? There's obviously a bit more work to do; the
 PoC
  currently only covers mysqli_query, but I thought this stage is an
  interesting point to throw it open to comments before working to
 complete
  it.
 
  Matt
 
  Hi Matt,
 
  PHP SQL injections are the #1 way PHP applications get hacked - and
 all SQL
  injections are the result of a developer either not understanding how
 to
  prevent SQL injection, or taking a shortcut because it's fewer
 keystrokes
  to do it a feels safe rather than is safe way.
 
  This may have been true at one point in time, but my own experience
  and the statistics collected by Dan Kaminsky of White Hat Security
  indicates that Cross-Site Scripting vulnerabilities are much more
  prevalent in 2015 than SQL Injection, especially in business
  applications. If Google has information that indicates that SQLi is
  still more prevalent than XSS, I'd love to see this data.
 
  

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-30 Thread Craig Francis
On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:

 But that is a perfect example of what I am talking about. You do not
 educate people by publishing the very thing that is wrong. You educate
 them by pointing out to them WHY the '?' was there in the first place.




I completely agree on education, and what I'm hoping for... and this is how we 
can educate everyone :-)

My suggestion for taints (not quite the same as the one from Matt or Wietse) 
was not to change the way good programs are created/executed, but simply an 
education device, which can also pick up mistakes that experienced developers 
make.

While my first post on this mailing list gives a better overview:

   http://news.php.net/php.internals/87207

The original implementation suggestion is at:

   https://bugs.php.net/bug.php?id=69886

You will see that it does nothing more than create notices to say erm, do you 
want to be doing this?.

This is something that only PHP can do, unless you can find a way of changing 
every single article / code example on the internet :-)

So, with your example... if you want to use a variable for a table/field 
prefix, that is perfectly fine... in fact, it won't need any changes, as the 
prefix will probably be hard coded as a string within a PHP script (something I 
called ETYPE_CONSTANT).

But if not (e.g. storing the prefix in an ini file?), then I've shown an 
example of how that can be handled with the proposed string_encoding_set 
function (something I should have probably called string_escaping_set)... which 
is simply to tell PHP that this one variable is already safe (something I can't 
see being needed very often).

Craig





On 30 Jul 2015, at 08:24, Lester Caine les...@lsces.co.uk wrote:

 On 29/07/15 16:11, Craig Francis wrote:
 I completely disagree... prepared statements are just as vulnerable, and so 
 are ORM's.
 
 You can push developers towards these solutions, and that would be good, but 
 you are completely blind if you think an uneducated developer won't do:
 
  if ($stmt = $mysqli-prepare(SELECT District FROM City WHERE Name= . 
 $_GET['name'])) {
  }
 
 But that is a perfect example of what I am talking about. You do not
 educate people by publishing the very thing that is wrong. You educate
 them by pointing out to them WHY the '?' was there in the first place.
 
 Since the taint extension only covers mysql and sqlite it's of little
 use if we manage to convert 'uneducated developer' to any of the more
 secure databases, and that was one of the reasons why mysql was dropped
 from being loaded by default. Once one starts from a base of
 parametrised sql queries the lax programming methods many mysql guides
 and books continue to push can be reversed. Throwing more bloat into php
 to create 'WTF' errors just adds to a new users frustration and annoys
 experienced users who have very good reasons for building queries using
 clean variables. MANY abstraction layers use variables to add prefixes
 to table names or fields.
 
 Educate ... don't nanny ...
 
 -- 
 Lester Caine - G8HFL
 -
 Contact - http://lsces.co.uk/wiki/?page=contact
 L.S.Caine Electronic Services - http://lsces.co.uk
 EnquirySolve - http://enquirysolve.com/
 Model Engineers Digital Workshop - http://medw.co.uk
 Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
 
 -- 
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php
 


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-29 Thread Craig Francis
On 29 Jul 2015, at 10:02, Lester Caine les...@lsces.co.uk wrote:

 The problem is removing all of the poor quality on-line guides and
 replacing them with ones which provide a mush better working model.
 Trying to get PHP too pick up a few edge cases is a poor use of time.




I completely disagree... prepared statements are just as vulnerable, and so are 
ORM's.

You can push developers towards these solutions, and that would be good, but 
you are completely blind if you think an uneducated developer won't do:

if ($stmt = $mysqli-prepare(SELECT District FROM City WHERE Name= . 
$_GET['name'])) {
}

And thats using a slightly edited example from:

http://php.net/manual/en/mysqli.prepare.php

It's a shame that Wietse suggested this solution in 2008, is incomplete, and 
does not seem to be going anywhere (I'm also tempted to say the implementation 
is slightly the wrong way around, but the theory is there).

Likewise the PECL extension from 2013.

http://pecl.php.net/package/taint

Matt, I realise I'm not a C programmer, and probably won't be able to help 
there, but if there is anything I can do, please let me know.

If you want to compare notes, my suggestion is at: 

http://news.php.net/php.internals/87207

Craig







On 29 Jul 2015, at 10:02, Lester Caine les...@lsces.co.uk wrote:

 On 28/07/15 18:33, Matt Tait wrote:
 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.
 
 If you want a safe and stable system ... don't use mysql ...
 The problem is removing all of the poor quality on-line guides and
 replacing them with ones which provide a mush better working model.
 Trying to get PHP too pick up a few edge cases is a poor use of time.
 
 -- 
 Lester Caine - G8HFL
 -
 Contact - http://lsces.co.uk/wiki/?page=contact
 L.S.Caine Electronic Services - http://lsces.co.uk
 EnquirySolve - http://enquirysolve.com/
 Model Engineers Digital Workshop - http://medw.co.uk
 Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
 
 -- 
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php
 


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-29 Thread Thomas Bley
Pierre Joye wrote on 28.07.2015 23:05:

 The
 On Jul 28, 2015 11:42 PM, Christoph Becker cmbecke...@gmx.de wrote:

 Rowan Collins wrote:

  On 28 July 2015 18:33:31 BST, Matt Tait matt.t...@gmail.com wrote:
  Hi all,
 
  I've written an RFC (and PoC) about automatic detection and blocking of
  SQL
  injection vulnerabilities directly from inside PHP via automated taint
  analysis.
 
  https://wiki.php.net/rfc/sql_injection_protection
 
  Have you searched the list archive and wiki for previous discussions
 and prototypes of variable tainting? The idea may well have some legs, but
 there might be some interesting points from previous discussions to note in
 your RFC.

 FWIW, there is the inactive Taint support for PHP[1] RFC.

 [1] https://wiki.php.net/rfc/taint
 
 Which is what should be done (global tainted mode) and not only for SQL.
 
 Unfiltered input can affect way more than only SQL. Environment, exec, etc
 are all potentially dangerous with unfiltered data.
 
 I fear it is an almost impossible task and may give a wrong signal,
 everything is safe of tainted mode is enabled.
 
 Cheers,
 Pierre
 


I think it's better to support parameter substitution and escaping directly in 
the extensions or the core functions:


Idea 1:

mixed mysqli_query_bind ( mysqli $link , string $query [, array $parameters [, 
int $resultmode = MYSQLI_STORE_RESULT ] ] )

e.g.

mysqli_query_bind($link, 'SELECT * FROM users WHERE usertype = ?', [$usertype]);

mysqli_query_bind($link, 'SELECT * FROM users WHERE id IN (?)', [[1,2,3]]);


Using mysqli_query_bind() means parameters are substituted in as (correctly) 
escaped strings and the result is run with mysqli_query().


and similar:

exec_bind ( string $command [, array $parameters [, array $output [, int 
$return_var ] ] ] )

echo exec_bind('ls ?', [$someDir]);

Using exec_bind() means parameters are substituted in as (correctly) escaped 
strings and the result is run with exec().


Those who want to secure their legacy code can use 
disable_functions=mysqli_query,exec and change the occurrences of both 
functions to the new bind functions.

If people still use echo exec_bind('ls '.$someDir), static code analysis can 
find it, similar to unsafe includes.

Regards
Thomas

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-29 Thread Lester Caine
On 28/07/15 18:33, Matt Tait wrote:
 What do you all think? There's obviously a bit more work to do; the PoC
 currently only covers mysqli_query, but I thought this stage is an
 interesting point to throw it open to comments before working to complete
 it.

If you want a safe and stable system ... don't use mysql ...
The problem is removing all of the poor quality on-line guides and
replacing them with ones which provide a mush better working model.
Trying to get PHP too pick up a few edge cases is a poor use of time.

-- 
Lester Caine - G8HFL
-
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-28 Thread Matt Tait
Hi all,

I've written an RFC (and PoC) about automatic detection and blocking of SQL
injection vulnerabilities directly from inside PHP via automated taint
analysis.

https://wiki.php.net/rfc/sql_injection_protection

In short, we make zend_strings track where their value originated. If it
originated as a T_STRING, from a primitive (like int) promotion, or as a
concatenation of such strings, it's query that can't have been SQL-injected
by an attacker controlled string. If we can't prove that the query is safe,
that means that the query is either certainly vulnerable to a SQL-injection
vulnerability, or sufficiently complex that it should be parameterized
just-to-be-sure.

There's also a working proof of concept over here:

http://phpoops.cloudapp.net/oops.php

You'll notice that the page makes a large number of SQL statements, most of
which are not vulnerable to SQL injection, but one is. The proof of concept
is smart enough to block that one vulnerable request, and leave all of the
others unchanged.

In terms of performance, the cost here is negligible. This is just basic
variable taint analysis under the hood, (not an up-front intraprocedurale
static analysis or anything complex) so there's basically no slow down.

PHP SQL injections are the #1 way PHP applications get hacked - and all SQL
injections are the result of a developer either not understanding how to
prevent SQL injection, or taking a shortcut because it's fewer keystrokes
to do it a feels safe rather than is safe way.

What do you all think? There's obviously a bit more work to do; the PoC
currently only covers mysqli_query, but I thought this stage is an
interesting point to throw it open to comments before working to complete
it.

Matt


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-28 Thread Pierre Joye
The
On Jul 28, 2015 11:42 PM, Christoph Becker cmbecke...@gmx.de wrote:

 Rowan Collins wrote:

  On 28 July 2015 18:33:31 BST, Matt Tait matt.t...@gmail.com wrote:
  Hi all,
 
  I've written an RFC (and PoC) about automatic detection and blocking of
  SQL
  injection vulnerabilities directly from inside PHP via automated taint
  analysis.
 
  https://wiki.php.net/rfc/sql_injection_protection
 
  Have you searched the list archive and wiki for previous discussions
and prototypes of variable tainting? The idea may well have some legs, but
there might be some interesting points from previous discussions to note in
your RFC.

 FWIW, there is the inactive Taint support for PHP[1] RFC.

 [1] https://wiki.php.net/rfc/taint

Which is what should be done (global tainted mode) and not only for SQL.

Unfiltered input can affect way more than only SQL. Environment, exec, etc
are all potentially dangerous with unfiltered data.

I fear it is an almost impossible task and may give a wrong signal,
everything is safe of tainted mode is enabled.

Cheers,
Pierre


Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-28 Thread Rowan Collins
On 28 July 2015 18:33:31 BST, Matt Tait matt.t...@gmail.com wrote:
Hi all,

I've written an RFC (and PoC) about automatic detection and blocking of
SQL
injection vulnerabilities directly from inside PHP via automated taint
analysis.

https://wiki.php.net/rfc/sql_injection_protection
 
Have you searched the list archive and wiki for previous discussions and 
prototypes of variable tainting? The idea may well have some legs, but there 
might be some interesting points from previous discussions to note in your RFC.

Also, 7.0 is already in beta, so your RFC will need to target 7.1 at the 
earliest.

Regards,
-- 
Rowan Collins
[IMSoP]

Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack

2015-07-28 Thread Christoph Becker
Rowan Collins wrote:

 On 28 July 2015 18:33:31 BST, Matt Tait matt.t...@gmail.com wrote:
 Hi all,

 I've written an RFC (and PoC) about automatic detection and blocking of
 SQL
 injection vulnerabilities directly from inside PHP via automated taint
 analysis.

 https://wiki.php.net/rfc/sql_injection_protection
  
 Have you searched the list archive and wiki for previous discussions and 
 prototypes of variable tainting? The idea may well have some legs, but there 
 might be some interesting points from previous discussions to note in your 
 RFC.

FWIW, there is the inactive Taint support for PHP[1] RFC.

[1] https://wiki.php.net/rfc/taint

-- 
Christoph M. Becker


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php