rfc:sql_injection_protection

Differences

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

Link to this comparison view

Both sides previous revisionPrevious revision
Next revision
Previous revision
rfc:sql_injection_protection [2015/07/28 16:23] – asd matttaitrfc:sql_injection_protection [2017/09/22 13:28] (current) – external edit 127.0.0.1
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 17: Line 18:
 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 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.
  
-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 systematically. And the damage caused to users and companies of a SQL-injection breach is completely and totally disproportionate to the effort in systematically defending against them properly.+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 systematically. And 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 blockSQL 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 "SafeConstsfor 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.txt · Last modified: 2017/09/22 13:28 by 127.0.0.1