# [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack - Thread: https://externals.io/message/87346 - Original archive: https://news-web.php.net/php.internals/87346 - Started by Matt Tait on 2015-07-28T17:33:31+00:00 - Messages: 45 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/87346.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #87346 — Matt Tait — 2015-07-28T17:33:31+00:00 URL: https://externals.io/message/87346#87346 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 --- ## #87348 — Rowan Collins — 2015-07-28T20:12:27+00:00 URL: https://externals.io/message/87346#87348 · In reply to: #87346 >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] --- ## #87350 — Christoph Becker — 2015-07-28T20:42:41+00:00 URL: https://externals.io/message/87346#87350 · In reply to: #87348 Rowan Collins wrote: > > > […] > > 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] -- Christoph M. Becker --- ## #87355 — Pierre Joye — 2015-07-28T21:05:49+00:00 URL: https://externals.io/message/87346#87355 · In reply to: #87350 The > > Rowan Collins wrote: > > > […] 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] 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 --- ## #87361 — Thomas Bley — 2015-07-29T01:26:57+00:00 URL: https://externals.io/message/87346#87361 · In reply to: #87355 Pierre Joye wrote on 28.07.2015 23:05: > The > > > […] > 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. > > […] > > 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 --- ## #87363 — Lester Caine — 2015-07-29T09:02:48+00:00 URL: https://externals.io/message/87346#87363 · In reply to: #87346 > 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 --- ## #87370 — Craig Francis — 2015-07-29T15:11:06+00:00 URL: https://externals.io/message/87346#87370 · In reply to: #87363 > 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 > > > […] > > 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 --- ## #87383 — Lester Caine — 2015-07-30T07:24:59+00:00 URL: https://externals.io/message/87346#87383 · In reply to: #87370 > 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 --- ## #87386 — Craig Francis — 2015-07-30T09:14:49+00:00 URL: https://externals.io/message/87346#87386 · In reply to: #87383 > 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 > > > […] > > 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 --- ## #87391 — Joe Watkins — 2015-07-30T12:14:24+00:00 URL: https://externals.io/message/87346#87391 · In reply to: #87386 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 wrote: > > > > […] > > > > > 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 > > > > > > > > > […] > and so are ORM's. > > […] > good, but you are completely blind if you think an uneducated developer > won't do: > > […] > Name=" . $_GET['name'])) { > > […] --- ## #87393 — Craig Francis — 2015-07-30T12:40:16+00:00 URL: https://externals.io/message/87346#87393 · In reply to: #87391 > 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 > 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 > > > > > > […] > > > > > 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 > > > > > > > > > […] --- ## #87396 — Xinchen Hui — 2015-07-30T12:47:58+00:00 URL: https://externals.io/message/87346#87396 · In reply to: #87391 Hey: > 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 > wrote: > > > […] -- Xinchen Hui @Laruence http://www.laruence.com/ --- ## #87400 — Scott Arciszewski — 2015-07-30T13:43:06+00:00 URL: https://externals.io/message/87346#87400 · In reply to: #87346 > 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 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 > Hey: > > > > […] > 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 > > […] > > > > -- > Xinchen Hui > @Laruence > http://www.laruence.com/ --- ## #87404 — Craig Francis — 2015-07-30T15:20:28+00:00 URL: https://externals.io/message/87346#87404 · In reply to: #87400 > 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 > > > […] > > Hi Matt, > > > […] > > 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 wrote: > > > > […] > > > 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 > > > > > > > > > > […] > 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 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. > 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. --- ## #87408 — Craig Francis — 2015-07-30T15:38:05+00:00 URL: https://externals.io/message/87346#87408 · In reply to: #87405 > 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 Thu, Jul 30, 2015 at 11:20 AM, Craig Francis > wrote: > > […] > > 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 wrote: > > > > […] > > > > 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). > > > > > > > > > > > > > […] > wrote: > > […] > completely missing) HTML escaping... have a look: > > […] > harder), but it certainly will pick up quite a lot of the issues (and it > wont break anything either, just help developers identify problems). > > […] > 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. > > […] > wrote: > > […] > wrote: > > […] > of SQL > > […] > it > > […] > as a > > […] > SQL-injected > > […] > safe, > > […] > SQL-injection > > […] > most of > > […] > concept > > […] > of the > > […] > basic > > […] > intraprocedurale > > […] > down. > > […] > all SQL > > […] > to > > […] > keystrokes > > […] > PoC > > […] > complete > > […] > all SQL > > […] > to > > […] > keystrokes > > […] --- ## #87431 — Craig Francis — 2015-07-31T10:40:37+00:00 URL: https://externals.io/message/87346#87431 · In reply to: #87409 > 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?). -------------------------------------------------- http://news.php.net/php.internals/87391 From: Joe Watkins Reply: Me (Craig Francis) Saying you agree with Pierre... I do as well, as Pierre was talking about Matt's solution. -------------------------------------------------- http://news.php.net/php.internals/87393 From: Me (Craig Francis) Reply: Joe Watkins Pointing out that my suggestion was different... I realise I'm now trying to derail Matt's original thread (this one seems to be getting some attention), but I really want to address these same issues. -------------------------------------------------- http://news.php.net/php.internals/87396 From: Xinchen Hui Reply: Joe Watkins Giving a quick status update on the original 2008 implementation... which I personally think is good, but could be made better by switching the logic around (I believe this will make the implementation easier, and avoid many edge cases). -------------------------------------------------- http://news.php.net/php.internals/87400 From: Scott Arciszewski Reply: Matt Tait Pointing out that Cross-Site Scripting is a bigger issue now... where I think that taints can address both issues (and more). He also says that prepared statements is a solved problem (something that I partially disagree with, and have explained above, but I realise that Scott is replying to Matt's original email, so may not have read that far yet). -------------------------------------------------- http://news.php.net/php.internals/87403 From: Me (Craig Francis) Reply: Xinchen Hui Just saying that I appreciate the work Xinchen is doing, and offering to help (assuming I can)... I do also go off on a bit of a tangent, as I think that if we invert the 2008 RFC to mark things when they are escaped (rather than tainting everything as they are created), then I think the implementation would be a lot easier. -------------------------------------------------- http://news.php.net/php.internals/87404 From: Me (Craig Francis) Reply: Scott Arciszewski Pointing out that I have a different suggestion. I also wanted to say that I don't think SQL injection is a solved problem, as I tried to explain earlier (which Scott may not have read yet)... but I certainly didn't want Scott to just sit back and think everything is solved now, so no need to discuss. -------------------------------------------------- http://news.php.net/php.internals/87405 From: Scott Arciszewski Reply: Me (Craig Francis) Skipping over my suggestion, and just saying that education is needed... which is what I'm also trying to do. But as PHP is the only thing that can be forced in front of a new developer, it is really the only common thing that can do that education (there is no way you can find and talk to every single new developer just by answering questions on Stack Overflow). And just as an example, college students leaning to write C programs in 2015 and creating buffer overflow vulnerabilities... they still make mistakes (even highly experienced developers still make these mistakes)... and you also have to recognise that not every programmer will go to a college, and that not all colleges actually teach their students about this (unfortunately). -------------------------------------------------- http://news.php.net/php.internals/87406 From: Ronald Chmara Reply: ??? I'm going to assume this is a reply to Matt's suggestion (or just the subject). And I agree, that suggestion would break every single application... hence why I'm not suggesting that approach. -------------------------------------------------- http://news.php.net/php.internals/87407 From: Me (Craig Francis) Reply: Ronald Chmara I'm just explaining that I've hijacked the email thread started by Matt, and that there is more to the discussion. -------------------------------------------------- http://news.php.net/php.internals/87408 From: Me (Craig Francis) Reply: Scott Arciszewski I am getting a little frustrated, and just want to move the discussion on. Unfortunately what you don't know is that I've been having this issue for too many years, and I'm getting quite annoyed that we aren't addressing these problems in a simple way. Instead, what I keep seeing is more complicated half solutions (e.g. ORMs and query builders, where typical website developers have no idea what the SQL/DB is doing, so vulnerabilities still creep in, and introduce other problems like n+1). The amount of times I've been to PHP conferences where the speaker is explaining some new way that they are doing OOP (or maybe even functional programming), and I'm sure they are really proud of their highly complex solution that few people in the audience understand (and no, I'm usually able to follow along, my experience is trying to explain what the talk was about to another audience member afterwards). But ultimately they haven't addressed anything todo with security, and it is often less performant (typical response "hey, get a faster computer"), it has instead taken them 3+ months to implement, has resulted in another 1 or 2 layers of abstraction that the typical developer does not understand (they just run it, and hope for the best). Then in 6 months time they will have moved on to a different complex solution, and can't be bothered to support the old one any more (because they just want something hard/complex to work on? I'm assuming it's an ego thing?). -------------------------------------------------- http://news.php.net/php.internals/87409 From: Joe Watkins Reply: Me (Craig Francis) Where we are today... where you are talking about the RFC (from 2008, or the one from Matt), not what I've just said about my suggestion (which, if you haven't read, is different). -------------------------------------------------- > 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 > > > > > > […] > > > > 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). > > > > > > > > > > > > > […] --- ## #87432 — Matt Tait — 2015-07-31T13:11:20+00:00 URL: https://externals.io/message/87346#87432 · In reply to: #87400 > > […] 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=search&description=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=main&dbg_sql&limit=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 will be "ignore", i.e. PHP will simply continue as in older versions. Developers who want to harden their code can run their website in "logging" mode to find and improve dangerous code in their codebase. Website administrators with heightened security awareness, or developers writing new code from scratch will be able to run the feature in "blocking" mode to prevent any possibility of SQL injection against their website. == What about applications that pass user-submitted SQL statements to the database by design? == This is accounted for in the RFC. Developers will be able to explicitly mark SQL queries as disabling the SQL-injection feature for the queries that explicitly warrant this (PHPMyAdmin being a good example). Again, this is only relevant if the website has been explicitly configured to use this feature. Matt > > > […] > SQL > > […] > SQL-injected > > […] > safe, > > […] > SQL-injection > > […] > of > > […] > concept > > […] > the > > […] > SQL > > […] > > Hi Matt, > > > […] > SQL > > […] > > 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 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?). > > -------------------------------------------------- > http://news.php.net/php.internals/87391 > From: Joe Watkins > Reply: Me (Craig Francis) > > Saying you agree with Pierre... I do as well, as Pierre was talking about > Matt's solution. > > -------------------------------------------------- > http://news.php.net/php.internals/87393 > From: Me (Craig Francis) > Reply: Joe Watkins > > Pointing out that my suggestion was different... I realise I'm now trying > to derail Matt's original thread (this one seems to be getting some > attention), but I really want to address these same issues. > > -------------------------------------------------- > http://news.php.net/php.internals/87396 > From: Xinchen Hui > Reply: Joe Watkins > > Giving a quick status update on the original 2008 implementation... which > I personally think is good, but could be made better by switching the logic > around (I believe this will make the implementation easier, and avoid many > edge cases). > > -------------------------------------------------- > http://news.php.net/php.internals/87400 > From: Scott Arciszewski > Reply: Matt Tait > > Pointing out that Cross-Site Scripting is a bigger issue now... where I > think that taints can address both issues (and more). > > He also says that prepared statements is a solved problem (something that > I partially disagree with, and have explained above, but I realise that > Scott is replying to Matt's original email, so may not have read that far > yet). > > -------------------------------------------------- > http://news.php.net/php.internals/87403 > From: Me (Craig Francis) > Reply: Xinchen Hui > > Just saying that I appreciate the work Xinchen is doing, and offering to > help (assuming I can)... I do also go off on a bit of a tangent, as I think > that if we invert the 2008 RFC to mark things when they are escaped (rather > than tainting everything as they are created), then I think the > implementation would be a lot easier. > > -------------------------------------------------- > http://news.php.net/php.internals/87404 > From: Me (Craig Francis) > Reply: Scott Arciszewski > > Pointing out that I have a different suggestion. > > I also wanted to say that I don't think SQL injection is a solved problem, > as I tried to explain earlier (which Scott may not have read yet)... but I > certainly didn't want Scott to just sit back and think everything is solved > now, so no need to discuss. > > -------------------------------------------------- > http://news.php.net/php.internals/87405 > From: Scott Arciszewski > Reply: Me (Craig Francis) > > Skipping over my suggestion, and just saying that education is needed... > which is what I'm also trying to do. > > But as PHP is the only thing that can be forced in front of a new > developer, it is really the only common thing that can do that education > (there is no way you can find and talk to every single new developer just > by answering questions on Stack Overflow). > > And just as an example, college students leaning to write C programs in > 2015 and creating buffer overflow vulnerabilities... they still make > mistakes (even highly experienced developers still make these mistakes)... > and you also have to recognise that not every programmer will go to a > college, and that not all colleges actually teach their students about this > (unfortunately). > > -------------------------------------------------- > http://news.php.net/php.internals/87406 > From: Ronald Chmara > Reply: ??? > > I'm going to assume this is a reply to Matt's suggestion (or just the > subject). > > And I agree, that suggestion would break every single application... hence > why I'm not suggesting that approach. > > -------------------------------------------------- > http://news.php.net/php.internals/87407 > From: Me (Craig Francis) > Reply: Ronald Chmara > > I'm just explaining that I've hijacked the email thread started by Matt, > and that there is more to the discussion. > > -------------------------------------------------- > http://news.php.net/php.internals/87408 > From: Me (Craig Francis) > Reply: Scott Arciszewski > > I am getting a little frustrated, and just want to move the discussion on. > > Unfortunately what you don't know is that I've been having this issue for > too many years, and I'm getting quite annoyed that we aren't addressing > these problems in a simple way. > > Instead, what I keep seeing is more complicated half solutions (e.g. ORMs > and query builders, where typical website developers have no idea what the > SQL/DB is doing, so vulnerabilities still creep in, and introduce other > problems like n+1). > > The amount of times I've been to PHP conferences where the speaker is > explaining some new way that they are doing OOP (or maybe even functional > programming), and I'm sure they are really proud of their highly complex > solution that few people in the audience understand (and no, I'm usually > able to follow along, my experience is trying to explain what the talk was > about to another audience member afterwards). > > But ultimately they haven't addressed anything todo with security, and it > is often less performant (typical response "hey, get a faster computer"), > it has instead taken them 3+ months to implement, has resulted in another 1 > or 2 layers of abstraction that the typical developer does not understand > (they just run it, and hope for the best). > > Then in 6 months time they will have moved on to a different complex > solution, and can't be bothered to support the old one any more (because > they just want something hard/complex to work on? I'm assuming it's an ego > thing?). > > -------------------------------------------------- > http://news.php.net/php.internals/87409 > From: Joe Watkins > Reply: Me (Craig Francis) > > Where we are today... where you are talking about the RFC (from 2008, or > the one from Matt), not what I've just said about my suggestion (which, if > you haven't read, is different). > > -------------------------------------------------- > > > > > > > > > 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 > wrote: > > > […] --- ## #87435 — Craig Francis — 2015-07-31T14:05:51+00:00 URL: https://externals.io/message/87346#87435 · In reply to: #87432 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 ''; 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 > > […] > > 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=search&description=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=main&dbg_sql&limit=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 will be "ignore", i.e. PHP will simply continue as in > older versions. Developers who want to harden their code can run their > website in "logging" mode to find and improve dangerous code in their > codebase. Website administrators with heightened security awareness, or > developers writing new code from scratch will be able to run the feature in > "blocking" mode to prevent any possibility of SQL injection against their > website. > > == What about applications that pass user-submitted SQL statements to the > database by design? == > This is accounted for in the RFC. Developers will be able to explicitly > mark SQL queries as disabling the SQL-injection feature for the queries > that explicitly warrant this (PHPMyAdmin being a good example). Again, this > is only relevant if the website has been explicitly configured to use this > feature. > > Matt > > > > > […] --- ## #87436 — Lester Caine — 2015-07-31T14:11:14+00:00 URL: https://externals.io/message/87346#87436 · In reply to: #87346 > 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 --- Next page: https://externals.io/message/87346.md?page=2