Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
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 Taitwrote: > 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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