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
