rfc:sql_injection_protection

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
Next revision
Previous revision
rfc:sql_injection_protection [2015/07/28 16:14]
matttait asd
rfc:sql_injection_protection [2017/09/22 13:28] (current)
Line 3: Line 3:
   * Date: 2015-07-22   * Date: 2015-07-22
   * Author: Matt Tait, matttait#​at#​google.com   * Author: Matt Tait, matttait#​at#​google.com
-  * Status: ​Draft+  * Status: ​Under discussion
   * First Published at: http://​wiki.php.net/​rfc/​sql_injection_protection   * First Published at: http://​wiki.php.net/​rfc/​sql_injection_protection
 +  * You can [[http://​phpoops.cloudapp.net/​oops.php||try it online]] ([[http://​phpoops.cloudapp.net/​oops.php?​action=main&​dbg_sql&​limit=4%20ohdear|spoiler]])
  
 ===== Background ===== ===== Background =====
Line 15: Line 16:
 And unlike hackers, developers are often given lots of bad information about how to defend against SQL injection. Many online guides will tell you to sanitize your queries with frighteningly broken tools - from regexes, to firewalls, to "​addslashes"​. [[http://​phpoops.cloudapp.net/​oops.php?​action=main&​dbg_sql&​limit=4%20mysql_real_escape_string_didnt_really_help_here|Even using the correct "SQL escape"​ function can be totally vulnerable to SQL injection]]. It is really easy for SQL-injections to hide in code that //feels// safe, but turns out not to be if you're not using parameterized queries for every dynamic SQL query. And unlike hackers, developers are often given lots of bad information about how to defend against SQL injection. Many online guides will tell you to sanitize your queries with frighteningly broken tools - from regexes, to firewalls, to "​addslashes"​. [[http://​phpoops.cloudapp.net/​oops.php?​action=main&​dbg_sql&​limit=4%20mysql_real_escape_string_didnt_really_help_here|Even using the correct "SQL escape"​ function can be totally vulnerable to SQL injection]]. It is really easy for SQL-injections to hide in code that //feels// safe, but turns out not to be if you're not using parameterized queries for every dynamic SQL query.
  
-It is also easy for developers ​to underestimate how attacker-controllable various strings in their complex web-application might be, or how readily exploitable hidden SQL injections are with modern tools. And with some regularity, "​unexploitable"​ SQL injections now become exploitable later, often when code is refactored and attacker-controllable strings find new routes to SQL statements buried deep inside the application.+It'​s ​easy to underestimate how attacker-controllable various strings in complex web-applications are; and how readily exploitable ​[[https://​www.owasp.org/​index.php/​Blind_SQL_Injection|hidden SQL injections]] are with modern tools. And with some regularity, "​unexploitable"​ SQL injections now become exploitable later, often when code is refactored and attacker-controllable strings find new routes to SQL statements buried deep inside the application.
  
-It's time PHP stopped letting this happen. ​Every decent web-application security tester knows there are two types of non-trivial PHP application:​ those that systematically parameterize every single dynamic statement, and those which are vulnerable ​to SQL-injection. ​The damage caused to users and companies of a SQL-injection breach ​is completely ​disproportionate to the couple ​of key-strokes saved by a developer ​in not parameterizing ​the statement.+Every big website ​that gets breached via a SQL injection attack thought that they were cleverer than hackers, and had invented a new way to defend against ​SQL injection ​attacks so they "​didn'​t need" to use parameterized SQL queries systematicallyAnd the damage caused to their company ​and users when they inevitably get breached ​is totally ​disproportionate to the amount ​of effort ​in simply making sure that they are systematically secure right from the start.
  
 This has to end. This has to end.
  
-This proposal is to add a new capability to PHP so that it can track vulnerable calls to SQL-based functions ​(initially "​mysql_query",​ but eventually all SQL functions). This allows PHP to distinguish strings that might be "​tainted"​ by remotely-obtained ​data (such as a $_REQUEST or database query result) from strings that are not (such as strings constructed from configuration files and string literals).+This proposal is to add a new capability to PHP so that it can track vulnerable calls to SQL-based functions. This allows PHP to distinguish strings that might be "​tainted"​ by remote ​data (such as a $_REQUEST or database query result) from strings that are not (such as strings constructed from configuration files and string literals), and detect when SQL-queries are unsafely constructed with remote data, or even when parameterized SQL queries use a non-constant query string.
  
-When PHP sees a function call to a potentially SQL-injectable ​query, ​PHP can then ignore, log, or take evasive action based on how the website administrator has configured their PHP.INI settings. ​This will allow companies to "​hard ​block" ​SQL queries ​across their infrastructure that might be vulnerable to SQL-injection.+When PHP sees a vulnerable ​query, ​it can then ignore, log, or take evasive action based on how the website administrator has configured their PHP.INI settings. ​Companies can then block all SQL injections ​across their entire website - even in extensions they haven'​t reviewed and files they'​ve forgotten about - with a one-line change in PHP.INI. Alternatively,​ they can opt-out of SQL-injection ​protection entirely, and run their existing code with no modifications or notices.
  
 I've hosted a proof of concept showing a vulnerable application [[http://​phpoops.cloudapp.net/​oops.php|here]]. The code you see is the code that's running. If you find the SQL injection vulnerability ([[http://​phpoops.cloudapp.net/​oops.php?​action=main&​dbg_sql&​limit=4%20oh_dear|spoiler]]),​ you'll see that the custom build of PHP blocks it from being exploited. I've hosted a proof of concept showing a vulnerable application [[http://​phpoops.cloudapp.net/​oops.php|here]]. The code you see is the code that's running. If you find the SQL injection vulnerability ([[http://​phpoops.cloudapp.net/​oops.php?​action=main&​dbg_sql&​limit=4%20oh_dear|spoiler]]),​ you'll see that the custom build of PHP blocks it from being exploited.
Line 29: Line 30:
 ===== Proposal (overview) ===== ===== Proposal (overview) =====
  
-This proposal is to modify the "​zend_string"​ type in Zend Core to "​track"​ whether a string ​contains dynamic parts that might be vulnerable to SQL injection, or whether it is provably ​constructed from strings that the server knows a hacker cannot control ​(such as constant config variables and string literals). ​We call these strings ​that attackers have no control over "SafeConsts" ​for the purposes of this proposal.+This proposal is to modify the "​zend_string"​ type in Zend Core to "​track"​ whether a string is statically ​constructed from string literals ​(like "​SELECT VERSION()"​) or concatenations of string literals ​(like $config['​table'​]='​foo';​ "​SELECT * from {$config['​table'​]}"​), or is dynamically generated in a way that might allow an attacker to control its valueIn this proposal we refer to these statically constructed ​strings ​as "safeconsts".
  
-Under this proposal, when a SQL builtin function (like "​mysql_query"​) runs, it can then "​ask"​ the SQL query string whether it is one of these SafeConst strings, or is a string ​that might be tainted with data from outside the PHP application (like via a $_GET parameter). If the parameter ​is recognized as a SafeConst, the query is fine and is executed as normal. If, on the other hand, the parameter is constructed dynamically,​ then we have found an unsafe use of SQL that is at best unsafe coding practices, and at worst, ​SQL-injection waiting to happenDepending on how the website ​is configured, we then either ​ignore the issue entirelyalert the developer via an E_WARNING and continue, or throw a SecurityError and refuse to execute ​the potentially vulnerable query.+When a SQL builtin function (e.g. "​mysql_query"​) runs, it can then "​ask"​ the SQL query string whether it is a "​safeconst" ​string. If it is, we know that the SQL query can be safely sent to the SQL database without fear that the SQL query has not been injected with malicious SQL code from hackerIf it is *not* a safeconst, we either ​logemit an E_WARNING and continue, or throw a "SecurityError ​exception" ​and abort the request, depending on the content of PHP.INI.
  
 === SafeConst === === SafeConst ===
  
 The logic for determining "​SafeConst"​-ness is as follows: The logic for determining "​SafeConst"​-ness is as follows:
-  * If the compiler sees a T_STRING, and that T_STRING ​does not contain ​variable replacement token, its corresponding zend_string ​is a SafeConst ​(i.e. "​Hello ​world" is a SafeConst, but "Hello $name" is not). +  * Any string literal (i.e. a T_STRING) is a safeconst string if it does not include any auto-eval parts (i.e. "Hello World" is safeconst). 
-  * If the compiler combines constant T_STRINGs together at compile timethe combination'​s corresponding zend_string ​is a safeconst. +  * A string literal with auto-eval parts is a safeconst if every auto-eval part evaluates to a safeconst ​(i.e. $name="​World", ​"​Hello ​$name" is a safeconst, but "​Hello ​{$_GET['​q'​]}" is not a safeconst). 
-  * $str1 . $str2 is a SafeConst if and only if $str1 is SafeConst ​string ​//​and// ​$str2 is a SafeConst ​string. +  * If $i is an Integer or FloatingPoint typeits string promotion ​is a safeconst ​string
-  * If define("​TABLE_NAME", ​$foois called, then TABLE_NAME ​is SafeConst string if and only if $foo is SafeConst string.  +  * If $i is an object type, its string promotion ​is a safeconst string ​if $i->​__toString() returns ​safeconst ​string
-  * Assigning ​a value into an array (by index or name), object, function parameter, return slot, local variable or global variable does not affect the SafeConst-ness of that variable. For example, mysqli_query("​SELECT * FROM " . $config["​tablename"​]) is fine, so long as $config["​tablename"​] was populated with a SafeConst.+  * $i.$is a safeconst only if $i and $j are both string ​types that are safeconsts, or are promoted to string types that are safeconsts
 +  * Defined variables are safeconsts if the value they were constructed from was a safeconst. I.e define("​TABLE_NAME", ​"​mytable"​); $i=TABLE_NAME ​yields ​safeconst in $i, but define("​TABLE_NAME",​ $_GET["​table"​]);​ $i=TABLE_NAME does not yield safeconst
 +  * Safeconstness follows a string instance around, i.e. assigning ​a value into an array (by index or name), object, function parameter, return slot, local variable or global variable does not affect the SafeConst-ness of that variable. For example, mysqli_query("​SELECT * FROM " . $config["​tablename"​]) is fine, so long as $config["​tablename"​] was populated with a SafeConst
 +  * Safeconstness does NOT follow string instance by value. For example $i = "​Hello";​ $i = $_GET["​q"​] leaves $i as *not* a safeconst, //even// if $_GET["​q"​] holds the value "​Hello"​.
  
 By design, SafeConstness is ***not** applied to the output of SQL-escape functions. Consider the following query: By design, SafeConstness is ***not** applied to the output of SQL-escape functions. Consider the following query:
-  mysqli_query("​SELECT * from USER where $id=" . mysqli_escape_query($_GET["​userid"​]))  +  mysqli_query("​SELECT * from USER where ID=" . mysqli_escape_query($_GET["​userid"​]))  
-This can be injected via page.php?​userid=1%20INJECT_HERE--. Even quoted forms +This can be injected via page.php?​userid=1%20INJECT_HERE--. Even using correct SQL-escapes within quotes ​can [[[[http://​shiflett.org/​blog/​2006/​jan/​addslashes-versus-mysql-real-escape-string|can be injected]] in a way PHP cannot verify at runtime 
-  mysqli_query("​SELECT * from USER where $id='"​ . mysqli_escape_query($_GET["​userid"​]) . "'"​) +  mysqli_query("​SELECT * from USER where ID='" ​mysqli_escape_query($_GET["​userid"​]) . "'"​) 
-can be injected using crazy [[http://​shiflett.org/​blog/​2006/​jan/​addslashes-versus-mysql-real-escape-string|charset bugs]], or if the SQL and the PHP server use different character sets. It is a critical security ​feature, not a bug, that this feature encourages ​developers to parameterize ​dynamic queries ​rather than building ​queries ​dynamically.+ 
 +It is a feature, not a bug, of this proposal that we are deliberately encouraging ​developers to parameterizerather than escape their SQL queries.
  
 === Example SQL-queries that are recognized as safe === === Example SQL-queries that are recognized as safe ===
Line 196: Line 201:
  
 ===== Unaffected PHP Functionality ===== ===== Unaffected PHP Functionality =====
-This change is designed to have the minimum user-visible impact for websites ​that do not use SQL, or who use it in a safe way.+Websites ​that already adopt security-best-practice of only issuing dynamic queries to their SQL database via parameterized SQL statements with constant parameter-strings will see no change when this proposal is adopted.
  
 ===== Future Scope ===== ===== Future Scope =====
Line 202: Line 207:
  
 ===== Proposed Voting Choices ===== ===== Proposed Voting Choices =====
-This requires a 50%+1 vote to be adopted.+This does not introduce any syntax changes to the PHP language, and therefore ​requires a 50%+1 vote to be adopted.
  
 ===== Patches and Tests ===== ===== Patches and Tests =====
rfc/sql_injection_protection.1438100073.txt.gz · Last modified: 2017/09/22 13:28 (external edit)