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/24 19:53]
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 =====
-[[https://​en.wikipedia.org/​wiki/​SQL_injection|SQL Injection]] vulnerabilities are a scourge on Internet security. [[https://​www.owasp.org/​index.php/​Top_10_2013-Top_10|According to OWASP]] it remains by far the most commonly ​exploited vulnerability category ​used to hack websites. SQL injections are ridiculously easy to exploit, and - depending on what data is stolen ​can often have a devastating and long-lasting impact on companies ​and users whose data is compromised.+[[https://​en.wikipedia.org/​wiki/​SQL_injection|SQL Injection]] vulnerabilities are a scourge on Internet security. ​They remain ​[[https://​www.owasp.org/​index.php/​Top_10_2013-Top_10|the ​Number 1 exploited vulnerability category ​online according ​to OWASP]], and as many as [[http://​www.esecurityplanet.com/​network-security/​two-thirds-of-u.s.-companies-were-breached-by-sql-injection-attacks-in-2013.html|two-thirds of US company ​data-breaches]] are ultimately due to SQL injection attacks.
  
-Given the severity and harm of SQL-injections,​ it is perhaps ​surprising that defending against SQL injection in the general case is a fully solved problem: if the website ensures that the dynamic parts of every single SQL query are sent as arguments to a constant [[https://​www.owasp.org/​index.php/​SQL_Injection_Prevention_Cheat_Sheet#​Defense_Option_1:​_Prepared_Statements_.28Parameterized_Queries.29|prepared statement]] (aka "​parameterized queries"​),​ the website is immediately and provably invulnerable to SQL injection via the application.+Given the severity and harm of SQL-injections,​ it may be surprising ​to hear that defending against SQL injection in the general case is a completely ​solved problem: if the website ensures that the dynamic parts of every single SQL query are sent as arguments to a constant [[https://​www.owasp.org/​index.php/​SQL_Injection_Prevention_Cheat_Sheet#​Defense_Option_1:​_Prepared_Statements_.28Parameterized_Queries.29|prepared statement]] (aka "​parameterized queries"​),​ the website is immediately and provably invulnerable to SQL injection via the application.
  
 But despite how easy this sounds in theory, hackers have one enormous advantage over developers: hackers need only find a single SQL-injection vulnerability in a website to fully compromise the server. Developers, in contrast, must successfully and systematically defend every single SQL query against an injection attack. That includes SQL in old code that nobody has looked at in years, SQL in third-party libraries and extensions, and SQL in that debug page that was never supposed to be uploaded to production websites, but somehow managed to get there nonetheless. But despite how easy this sounds in theory, hackers have one enormous advantage over developers: hackers need only find a single SQL-injection vulnerability in a website to fully compromise the server. Developers, in contrast, must successfully and systematically defend every single SQL query against an injection attack. That includes SQL in old code that nobody has looked at in years, SQL in third-party libraries and extensions, and SQL in that debug page that was never supposed to be uploaded to production websites, but somehow managed to get there nonetheless.
  
-Perhaps worst of allit is frighteningly ​easy for SQL-injections to hide in code that //feels// safe, but turns out not to be completely unsafe, or rely on magic "stop SQL injection"​ functions written by the local office "​security guru", only to find out too late that they aren't all that safe at all.+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 regexesto 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 ​the capability of hackers to get crafted input into strange places, or exploit weird and SQL injections buried deep in website back-end code. For example, hackers can exploit a website via [[https://​haiderm.com/​referer-header-based-blind-sql-injection-explained-example/​|via headers]] (like $_SERVER["​HTTP_HOST"​] ​and $_SERVER["​X-Forwarded-For"​]). Hackers can SQL-inject in backend services where the hacker ​[[https://​www.owasp.org/​index.php/​Blind_SQL_Injection|can't even see any output or error message]]. Hackers can [[https://​haiderm.com/​second-order-sql-injection-explained-example/​|reflect data back at the database]]. Websites get hacked when developers use [[http://​security.stackexchange.com/​questions/​3611/​sql-injection-why-isnt-escape-quotes-safe-anymore|the wrong SQL-escape function]]when they  [[http://​stackoverflow.com/​questions/​5741187/​sql-injection-that-gets-around-mysql-real-escape-string|use the right SQL-escape function]]and (depressingly ​oftenwhen they try and [[https://​www.owasp.org/​images/​d/​d4/​OWASP_IL_2007_SQL_Smuggling.pdf|build their own escape function]].+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 toolsAnd 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.
  
-In shortevery dynamic ​SQL statement that isn'​t ​parameterized SQL statement with a constant SQL parameter puts the integrity of the website ​and user data at extraordinary risk just to shave a couple ​of seconds off the developer'​s time when writing the website. For precisely this reason, most security-conscious companies already institute a global policy of banning all SQL-statements with dynamic query strings, and most web-application security guidelines include a ban on unparameterized dynamic SQL statements+Every big website that gets breached via a SQL injection attack thought that they were cleverer than hackersand 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 proposal gives website owners a way of enforcing this sensible security precaution across the entire website as an easy-to-enable configuration setting. Blocking insecure use of SQL - and warning developers why they should and how they can convert their code to be safe - would have an enormously positive influence on the security of webapplications written in PHP, and the security of user data on the web+This has to end.
  
-Writing code that can be SQL-injected ​in 2015 isn'​t ​acceptable. ​PHP should help developers to detect ​and systematically eliminate this category ​of vulnerability from the webonce and for all.+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 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.
  
 ===== 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 if the charset on the SQL server and the PHP server get out-of-sync. ​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 192: 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 198: 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.1437767592.txt.gz · Last modified: 2017/09/22 13:28 (external edit)