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:31] – 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 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.1438101083.txt.gz · Last modified: 2017/09/22 13:28 (external edit)