Req #69886 [NEW]: Addressing problems with escaping (security)
| From: | craig at craigfrancis dot co dot uk | 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