Req #69886 [NEW]: Addressing problems with escaping (security)

From: Date: Fri, 19 Jun 2015 15:35:14 +0000
Subject: Req #69886 [NEW]: Addressing problems with escaping (security)
Groups: php.bugs 
Request: Send a blank email to php-bugs+get-193703@lists.php.net to get a copy of this message
From: craig at craigfrancis dot co dot uk Operating system: PHP version: Irrelevant Package: Strings related Bug Type: Feature/Change Request Bug description:Addressing problems with escaping (security) Description: ------------ The major security vulnerabilities with websites are STILL Injection/XSS related... see OWASP top 10 for 2010 and 2013 (A1 and A3). Many developers still have no idea about the security vulnerabilities when they do things like: echo $_GET['name']; Prepared statements try (and fail) to solve this: $mysqli->prepare('SELECT name FROM table WHERE id = ' . $_GET['id']); ORM's try (and fail) to address this: $conn->createQueryBuilder() ->select('u.id') ->addSelect('p.id') ->from('users', 'u') ->leftJoin('u', 'phonenumbers', ' u.id = p.user_id AND p.type = ' . $_GET['type']); Templating systems try (and fail) to fix this: {{ var }} As an aside... "|escape" or "autoescape" is still not the default in Twig? -------------------------------------------------- I'm proposing an error_reporting() mode (or extending E_STRICT) that when be enabled, should help identify (highlight?) the most common mistakes. Importantly, the developer won't need to see any of this... they just enable it, and the logs get filled with PHP notices, telling them to fix these mistakes. -------------------------------------------------- Internally (zval), every PHP string gets marked with a escaping type. A simple string defined in the PHP code itself (T_CONSTANT_ENCAPSED_STRING) would be a ETYPE_CONSTANT... this is a special type of string, where we know the developer has typed it in the PHP code themselves (safe... ish). Any string from the outside world (request parameters, database, files, etc) would be an ETYPE_PLAIN... this is pretty much the default for all new strings. Any string that is encoded/escaped, will have its own ETYPE... e.g. the return strings from these functions would be: htmlentities() -> ETYPE_HTML mysqli_real_escape_string() -> ETYPE_SQL urlencode() -> ETYPE_URL escapeshellarg() -> ETYPE_SHELL escapeshellcmd() -> ETYPE_SHELL preg_quote() -> ETYPE_PREG PHP will be configured (ini_set) with an output type... as we are normally creating websites, the default output would be ETYPE_HTML, but a CLI script might be ETYPE_PLAIN. And there would need to be a function to override the type for special cases: $result = mysqli_query('SELECT name, bio_html FROM person WHERE id = 3'); if ($row = mysqli_fetch_assoc($result)) { $name_html = htmlentities($row['name']); $bio_html = string_encoding_set($row['bio_html'], ETYPE_HTML); // Assume bio_html has already // been though htmlpurifier :-) } -------------------------------------------------- When concatenating/printing the strings, PHP would do a simple type check, e.g. $a = 'Hi ' . $_GET['name']; // ETYPE_CONSTANT + ETYPE_PLAIN // This is fine, // $a is now ETYPE_PLAIN. echo $a; // Log error! // The output is ETYPE_HTML, // We can't echo a ETYPE_PLAIN! $b = $_GET['c'] . htmlentities($_GET['d']); // ETYPE_PLAIN + ETYPE_HTML // Log error! // Cannot mix types like this. $sql = 'SELECT * FROM table WHERE id = ' . $_GET['id']; // ETYPE_CONSTANT + ETYPE_PLAIN + ETYPE_CONSTANT // This is fine (for now). // $sql is now ETYPE_PLAIN. mysqli_query(sql); // Log error! // Cannot accept ETYPE_PLAIN, // It must be ETYPE_SQL! $sql = 'SELECT * FROM table WHERE id = "' . mysqli_real_escape_string($_GET['id']) . '"'; // ETYPE_CONSTANT + ETYPE_SQL + ETYPE_CONSTANT // This is fine. // $sql is now ETYPE_SQL. mysqli_query(sql); // Good :-) -------------------------------------------------- The intention is to identify when escaped strings are being mixed with un-escaped strings... and when un-escaped strings are incorrectly passed to certain functions (or the output). This isn't a complete solution, but I believe it will catch a lot of errors the other solutions do not (cannot?) address. I am also aware of ValueObjects... I'm fairly sure these would only get you half way there, and suffer from the same problems as the existing solutions. Some of the problems (which you might be able to come up with solutions for), are listed below in the comments. -- Edit bug report at https://bugs.php.net/bug.php?id=69886&edit=1 -- Try a snapshot (PHP 5.4): https://bugs.php.net/fix.php?id=69886&r=trysnapshot54 Try a snapshot (PHP 5.5): https://bugs.php.net/fix.php?id=69886&r=trysnapshot55 Try a snapshot (trunk): https://bugs.php.net/fix.php?id=69886&r=trysnapshottrunk Fixed in SVN: https://bugs.php.net/fix.php?id=69886&r=fixed Fixed in release: https://bugs.php.net/fix.php?id=69886&r=alreadyfixed Need backtrace: https://bugs.php.net/fix.php?id=69886&r=needtrace Need Reproduce Script: https://bugs.php.net/fix.php?id=69886&r=needscript Try newer version: https://bugs.php.net/fix.php?id=69886&r=oldversion Not developer issue: https://bugs.php.net/fix.php?id=69886&r=support Expected behavior: https://bugs.php.net/fix.php?id=69886&r=notwrong Not enough info: https://bugs.php.net/fix.php?id=69886&r=notenoughinfo Submitted twice: https://bugs.php.net/fix.php?id=69886&r=submittedtwice register_globals: https://bugs.php.net/fix.php?id=69886&r=globals PHP 4 support discontinued: https://bugs.php.net/fix.php?id=69886&r=php4 Daylight Savings: https://bugs.php.net/fix.php?id=69886&r=dst IIS Stability: https://bugs.php.net/fix.php?id=69886&r=isapi Install GNU Sed: https://bugs.php.net/fix.php?id=69886&r=gnused Floating point limitations: https://bugs.php.net/fix.php?id=69886&r=float No Zend Extensions: https://bugs.php.net/fix.php?id=69886&r=nozend MySQL Configuration Error: https://bugs.php.net/fix.php?id=69886&r=mysqlcfg

« previous php.bugs (#193703) next »