Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Matt, You are of course welcome to disagree with the overwhelming body of security advice that parameterized queries are the correct, secure way to prevent SQL injection. In that case, you only need to not enable this feature. This feature is off-by-default, and only attempts to help secure webapplications and webdevelopers who do (or are required, for example by PCI compliance standards) to adopt this security-best-practice to ensure that they do so systematically across their entire website. You seem to misunderstand me. I'm *not* saying that you shouldn't use parameterized queries. Nor that they are not one of the best tools available. I am simply pointing out that they are not a sure-fire one-stop solution. There is a lot more that goes into it than simply using prepare/bind. As indicated by the two cases I talked about (structural elements not being supported and implementation quirks) as well as others (DOS attacks from wildcards in malformed query parameters, type validation, etc). This is not to say that we should avoid PQ, but that we should recognize that it's not enough to just use one thing and forget about everything else. Anthony PS: w3schools is NOT w3c. And their content is probably the single largest source of security vulnerabilities for PHP, so citing them in a security discussion is more than a little ironic. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Recap - Core functions throwing exceptions in PHP7
On Thu, 6 Aug 2015 00:55 Scott Arciszewski sc...@paragonie.com wrote: All, I'd like to move the conversation towards a decision regarding PRs 1397 and 1398. These decisions are blocking random_compat as well as a security enhancement to random_bytes (merge conflicts are *the worst*). Here's a quick recap Arguments: 1. Consistency is more important than security. random_* should be consistent with the rest of PHP (sans intdiv()). They should return false and emit an E_WARNING or E_ERROR warning (the latter is if we want it to fail closed). It's the responsibility of the developer to know this can happen and explicitly check for it. Don't throw anything. 2. Security is more important than consistency. Placing more responsibility on the developer increases the likelihood of an implementation error. We should aim for compatible usability with rand() and mt_rand() for random_int(), which never return false. For random_int() and random_bytes(), should PHP be unable to generate a random value (no random device available, file descriptor exhaustion, etc.) an exception should be thrown. If the developer wants to handle it gracefully, they can place it in try/catch blocks (which raising errors make messy). Otherwise, the default state is to fail closed (i.e. terminate script execution). Open Questions: 1. Under what conditions should an Exception be thrown, and which should throw an Error instead? Did I miss any? I'm in favor of throwing *something*. As for the particulars of what should be an Exception and what should be an Error, I don't have a horse in this race. Exceptions already existed and Errors were already accepted in the Throwable RFC, so I don't believe this warrants another RFC and putting this decision off until 7.1. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php We also -need- a consensus in place because we have more security sensitive features targeting 7.1 and we don't want to have this argument again. Throwing exception's may not be consistent right now, but it certainly should be in the future. The transition to a new consistency has to start somewhere.
Re: [PHP-DEV] Recap - Core functions throwing exceptions in PHP7
On Aug 6, 2015, at 3:52 AM, Niklas Keller m...@kelunik.com wrote: Scott, could you setup a RFC with a vote, so we can decide? Nikita proposed those two options: 1) Error is to be used in cases where an error is attributable to programmer mistake. 2) Error signifies a failure condition that the programmer is discouraged (and unlikely to want) to handle. It should only be dealt with at the top level. I'm in favor of 2), I would phrase it like: Error should be used if a repetitive call with the same input parameters would _permanently_ result in a failure, otherwise Exception. Regards, Niklas I’m in favor of 1), as this was my original intention of the Error branch of exceptions. Errors should be attributable to a programming mistake that should be corrected. Exception should be thrown for unexpected conditions that are not due to programmer error, but instead things like IO or permission errors. I think this is how exceptions thrown from core functions (and all functions or methods in extensions) should be organized. Based on this interpretation, random_int() should throw an Error if $min $max and random_bytes() should throw an Error if $length = 0. random_bytes() and random_int() should throw an Exception if random data cannot be generated. Another quote from Nikita’s message to the prior thread: Another interesting aspect are the zpp calls. Currently these will throw a warning and return NULL in weak mode and throw a TypeError in strict mode. I wonder whether we should be using zpp_throw for new functions, so a TypeError is also thrown in weak mode. Continuing to use the old warning+NULL behavior was a BC concern, which we don't have for new functions. The reason why I think this is worth considering, is that if all other error conditions of a function already throw, then ordinary zpp will still add one case where a different type is returned on error. This makes random_int from a function returning int, to a function returning int|null. I would also be in favor of throwing TypeError from zpp calls in new functions (and quite frankly, from zpp calls in all functions, including old functions). Regards, Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Hi Matt,
On Thu, Aug 6, 2015 at 12:46 PM, Matt Tait matt.t...@gmail.com wrote:
I'll take a few of your points in turn.
With regards to the fact that not all SQL queries are directly
parameterizable, this is true. Structural parts of a query, such as table
names, column names and complex conditions are hard to parameterize with
vanilla prepared statements, and many developers like to abstract some of
these structural parts of a SQL query into config files, and append
additional conditional constraints into the query at runtime based on user
input.
This feature addresses this head on. So long as the structural parts of the
prepared statement -- that is, table names, database names and column names
-- are not themselves attacker-controlled (I can't think of a valid reason
whey they would be), this feature is happy for developers to concatenate
them into a query string. For example, the following is not detected by the
feature as dangerous, because the query (whilst dynamically constructed) is
the linear concatenation of string literals, and so is a safeconst.
$query = SELECT * from {$config['tablename']} WHERE id=?;
if(isset($_GET[filterbycolor]))
$query .= AND color=?;
do_prepared_statement($query, array(id = $_GET[id] color =
$_GET[color]));
If you would like to prevent SQL injection via Identifiers, how about
extend prepared query like
$query = SELECT * from @ WHERE id=?;
if(isset($_GET[filterbycolor]))
$query .= AND color=?;
do_prepared_statement($query, array(id = $_GET[id] color =
$_GET[color]), array($config['tablename']));
where @ indicates identifier placeholder. The char could be anything that
will not violate SQL syntax. This could be implemented in user land as
there is no standard/native API for identifier placeholder.
Even if there is identifier placeholder, SQL keyword remains.
So to be perfect, you'll need another place holder for SQL keywords.
There is no escaping for SQL keywords and it has to be validation.
e.g. ORDER BY {$_GET['order']}
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
On Fri, Aug 7, 2015 at 10:29 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:
Even if there is identifier placeholder, SQL keyword remains.
So to be perfect, you'll need another place holder for SQL keywords.
There is no escaping for SQL keywords and it has to be validation.
e.g. ORDER BY {$_GET['order']}
Oops the last line should be
e.g. ORDER BY col {$_GET['order']}
BTW, instead of improving PHP, users are better to request identifier
escape API
to DB developers like PQescapeIdentifier() in PostgreSQL's client library.
IMO.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Thanks for the feedback Anthony, This feature specifically addresses the points you raise; the feature allows parameterized queries constructed with structural parts of the query inserted from configuration variables, so long as the resulting query is a safe-const as defined by this RFC. If you can come up with an example of a query that either (a) is vulnerable to a SQL injection attack and this feature wrongly does not detect it as such or (b) a query that cannot be expressed using parameterized queries where the parameter is a safe-const as defined by this RFC, I'd be genuinely interested to take a look. Hope that clarifies, Matt On Aug 6, 2015, at 14:34, Anthony Ferrara ircmax...@gmail.com wrote: Matt, You are of course welcome to disagree with the overwhelming body of security advice that parameterized queries are the correct, secure way to prevent SQL injection. In that case, you only need to not enable this feature. This feature is off-by-default, and only attempts to help secure webapplications and webdevelopers who do (or are required, for example by PCI compliance standards) to adopt this security-best-practice to ensure that they do so systematically across their entire website. You seem to misunderstand me. I'm *not* saying that you shouldn't use parameterized queries. Nor that they are not one of the best tools available. I am simply pointing out that they are not a sure-fire one-stop solution. There is a lot more that goes into it than simply using prepare/bind. As indicated by the two cases I talked about (structural elements not being supported and implementation quirks) as well as others (DOS attacks from wildcards in malformed query parameters, type validation, etc). This is not to say that we should avoid PQ, but that we should recognize that it's not enough to just use one thing and forget about everything else. Anthony PS: w3schools is NOT w3c. And their content is probably the single largest source of security vulnerabilities for PHP, so citing them in a security discussion is more than a little ironic. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Move to Fast ZPP?
Hi Levi, - Original Message - From: Levi Morrison Sent: Thursday, August 06, 2015 Don't know about Windows now... Visual Studio 2008 and 2012 (not much difference) are NOT optimizing away the code (other times it was GCC with issues). :-/ Not sure why. Of course they don't support the necessary compound literals anyway, but I was just testing a manual case... I'll have to try and check 2015 version soon. VS 2015 supports compound literals, I believe. Yes, VS 2013 added, AFAIK. But I was just talking about optimizing the code that would use them. A compound literal (void *[]), derived from variadic macro, is needed to transform zend_parse_parameters() into a non-varargs inline function call (since va_arg usage prevents inlining). The problem in the earlier VS versions, which I simulated manually without compound literals, is that it doesn't want to optimize away all the inlined stuff, leaving only the result. Definitely can't have something creating several KB of extra instructions! :-) So unless they improved something in the newest versions, this wouldn't be able to be used there, unless I can work around it somehow. I don't know if macros, instead of inline functions, inside the main inline function would help or not... Although, it *would* simplify a couple tiny parts if didn't need to support MSVC... First, detecting a string *literal* (99% of usage): could just use __builtin_constant_p() instead of a macro trick. And second, using a variable length array, with size set by, well, a variable, which I think VS still doesn't support from C99, right? (It's for a variable in this case, but always the same, so should optimize like a compile-time size.) Thanks, Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Move to Fast ZPP?
On Aug 6, 2015 1:49 PM, Matt Wilmas php_li...@realplain.com wrote: Hi Levi, - Original Message - From: Levi Morrison Sent: Thursday, August 06, 2015 Don't know about Windows now... Visual Studio 2008 and 2012 (not much difference) are NOT optimizing away the code (other times it was GCC with issues). :-/ Not sure why. Of course they don't support the necessary compound literals anyway, but I was just testing a manual case... I'll have to try and check 2015 version soon. VS 2015 supports compound literals, I believe. Yes, VS 2013 added, AFAIK. But I was just talking about optimizing the code that would use them. A compound literal (void *[]), derived from variadic macro, is needed to transform zend_parse_parameters() into a non-varargs inline function call (since va_arg usage prevents inlining). The problem in the earlier VS versions, which I simulated manually without compound literals, is that it doesn't want to optimize away all the inlined stuff, leaving only the result. Definitely can't have something creating several KB of extra instructions! :-) So unless they improved something in the newest versions, this wouldn't be able to be used there, unless I can work around it somehow. I don't know if macros, instead of inline functions, inside the main inline function would help or not... Although, it *would* simplify a couple tiny parts if didn't need to support MSVC... First, detecting a string *literal* (99% of usage): could just use __builtin_constant_p() instead of a macro trick. And second, using a variable length array, with size set by, well, a variable, which I think VS still doesn't support from C99, right? (It's for a variable in this case, but always the same, so should optimize like a compile-time size.) If you have a piece of self contained code to test it, we can then add it to the VC test repo to valid what we can support with 7+.
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Hi Matt, Am 06.08.2015 um 05:46 schrieb Matt Tait: With regards to the fact that not all SQL queries are directly parameterizable, this is true. Structural parts of a query, such as table names, column names and complex conditions are hard to parameterize with vanilla prepared statements, and many developers like to abstract some of these structural parts of a SQL query into config files, and append additional conditional constraints into the query at runtime based on user input. This feature addresses this head on. So long as the structural parts of the prepared statement -- that is, table names, database names and column names -- are not themselves attacker-controlled (I can't think of a valid reason whey they would be), this feature is happy for developers to concatenate them into a query string. For example, the following is not detected by the feature as dangerous, because the query (whilst dynamically constructed) is the linear concatenation of string literals, and so is a safeconst. and how exactly do you guarantee the structural part from a configuration is not attacker-controlled? The config may come from a json file, from a database, from APCu, etc. and may have been inserted/generated previously from unsafe user input. If I understand you right, these parts will not be tainted and thus will give a wrong feeling of safety to the unskilled/unknowing programmer. Or did I miss that part in the RFC? Greets Dennis -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Generating release verification stub
Hi, as we put several verification info into the announcement mails, and after doing it a couple of times manually, I've invented this quick solution. https://gist.github.com/weltling/2d2972aa5325ee3b530c I would suggest to take it into the scripts/ and to adjust the corresponding release process document part. It's a bit raw yet, but does the job and can be improved later. This way we can standardize the matter and avoid c/p mistakes. Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] zend_string or not?
Hi all, Is there zend_string usage guideline? I'm wondering if zend_string is used where it is appropriate. Once we release PHP7, adopting zend_string for PHPAPI functions become difficult. (We have to keep legacy API or it will be 3rd party module author's headache if we change this with minor version up.) Evaluation for all PHPAPI functions that have char * parameter is finished? If not, we are better to do this now at least for core functions. LXR output seems there are number of core functions that may use zend_string. http://lxr.php.net/search?q=PHPAPIdefs=refs=path=hist=project=PHP_TRUNK Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Benchmark Results for PHP Master 2015-08-06
Results for project php-src-nightly, build date 2015-08-06 10:30:35+03:00 commit: 2eaf28367dd5da282156f567f8dbc031a4dbb2c2 revision_date: 2015-08-05 12:04:26-07:00 environment:Haswell-EP cpu:Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz 2x18 cores, stepping 2, LLC 45 MB mem:128 GB os: CentOS 7.1 kernel: Linux 3.10.0-229.4.2.el7.x86_64 Note: Baseline results were generated using release php-7.0.0beta1, with hash ad8a73dd55c087de465ad80e8715611693bb1460 from 2015-07-07 16:02:13+00:00 benchmark executable unit change since change since yesterday php-7.0.0beta1 Wordpress 4.2.2 cgi -T1 php opc=on fps 0.78% 1.57% Drupal 7.36 cgi -T1 php opc=on fps 1.37% 2.86% MediaWiki 1.23.9 cgi -T5000 php opc=on fps 1.34% 2.52% bench.php cgi -T1 php opc=on sec -0.98% -3.11% micro_bench.php cgi -T1 php opc=on sec -4.48% -1.48% mandelbrot.php cgi -T1 php opc=on sec -1.49% -2.32% Our lab does a nightly source pull and build of the PHP project and measures performance changes against the previous stable version and the previous nightly measurement. This is provided as a service to the community so that quality issues with current hardware can be identified quickly. Intel technologies' features and benefits depend on system configuration and may require enabled hardware, software or service activation. Performance varies depending on system configuration. No license (express or implied, by estoppel or otherwise) to any intellectual property rights is granted by this document. Intel disclaims all express and implied warranties, including without limitation, the implied warranties of merchantability, fitness for a particular purpose, and non-infringement, as well as any warranty arising from course of performance, course of dealing, or usage in trade. This document may contain information on products, services and/or processes in development. Contact your Intel representative to obtain the latest forecast, schedule, specifications and roadmaps. The products and services described may contain defects or errors known as errata which may cause deviations from published specifications. Current characterized errata are available on request. (C) 2015 Intel Corporation. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Recap - Core functions throwing exceptions in PHP7
Scott, could you setup a RFC with a vote, so we can decide? Nikita proposed those two options: 1) Error is to be used in cases where an error is attributable to programmer mistake. 2) Error signifies a failure condition that the programmer is discouraged (and unlikely to want) to handle. It should only be dealt with at the top level. I'm in favor of 2), I would phrase it like: Error should be used if a repetitive call with the same input parameters would _permanently_ result in a failure, otherwise Exception. Regards, Niklas
Re: [PHP-DEV] Move to Fast ZPP?
Hi Pierre, - Original Message - From: Pierre Joye Sent: Thursday, August 06, 2015 On Aug 6, 2015 1:49 PM, Matt Wilmas php_li...@realplain.com wrote: Hi Levi, - Original Message - From: Levi Morrison Sent: Thursday, August 06, 2015 Don't know about Windows now... Visual Studio 2008 and 2012 (not much difference) are NOT optimizing away the code (other times it was GCC with issues). :-/ Not sure why. Of course they don't support the necessary compound literals anyway, but I was just testing a manual case... I'll have to try and check 2015 version soon. VS 2015 supports compound literals, I believe. Yes, VS 2013 added, AFAIK. But I was just talking about optimizing the code that would use them. A compound literal (void *[]), derived from variadic macro, is needed to transform zend_parse_parameters() into a non-varargs inline function call (since va_arg usage prevents inlining). The problem in the earlier VS versions, which I simulated manually without compound literals, is that it doesn't want to optimize away all the inlined stuff, leaving only the result. Definitely can't have something creating several KB of extra instructions! :-) So unless they improved something in the newest versions, this wouldn't be able to be used there, unless I can work around it somehow. I don't know if macros, instead of inline functions, inside the main inline function would help or not... Although, it *would* simplify a couple tiny parts if didn't need to support MSVC... First, detecting a string *literal* (99% of usage): could just use __builtin_constant_p() instead of a macro trick. And second, using a variable length array, with size set by, well, a variable, which I think VS still doesn't support from C99, right? (It's for a variable in this case, but always the same, so should optimize like a compile-time size.) If you have a piece of self contained code to test it, we can then add it to the VC test repo to valid what we can support with 7+. Sure! Simply: size_t num = 4; char foo[num]; I wondered about emulating with _alloca(), but a quick check (in 2008!) shows that it doesn't optimize it out even with a true constant value... - Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] PHP 7.0.0beta3 is available
Hi, The third beta for 7.0.0 was just released and can be downloaded from: https://downloads.php.net/~ab/ The Windows binaries are available at http://windows.php.net/qa/ This release contains fixes for 33 reported bugs, 11 of which are security related, and altogether over 200 commits with various improvements. Please test it carefully, and report any bugs in the bug system. The next scheduled release is going to be tagged on August 18th and released on August 20th. It will be RC 1. Below is the verification information for the downloads: php-7.0.0beta3.tar.bz2 SHA256 hash: 7dcb7070403e7c2bfb4936dfa4c1f343f4071d2957d563689e51a2899b04a6b0 PGP signature: -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAABAgAGBQJVwdsTAAoJELyqMOqcDVdjVa8H/1osHz9tP7AKqZASywzEcPlS gfKqOIGnu7cWZfvG55iKBnKTwN22xtUVXq9g7XwOKYl/sXJK/D9izFSPBzkxEJJL zzD1rEXS4oSeXuBfhOVxHo7Z24iFyexnwdzx0Vh3pzwVWaKWnteqJddon6vkOUBf 7wQzHkndY+tT1e/nRi9O1tjg2C5MviyaPP0zofRu1eMEgD0fjp5+HwWaGeGRCKv3 Fl0hhB6im7P85Uz+8B6l5WQlEcd2dZ0sQmIYYhdD9oiQa48+1r5g8Sn6QHVgwr0k F/hXNKlzcn7pSaSK7Q4N8dSP0xlDmc4Bj8BRjQcUeMOR7ujSehF1nd6IYzkPJCc= =PVjn -END PGP SIGNATURE- php-7.0.0beta3.tar.gz SHA256 hash: 8fb5448fb116581a2cf8514e8ed3f54c4da0b0dfddf61e21a5bdb0945604494e PGP signature: -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAABAgAGBQJVwdsgAAoJELyqMOqcDVdj4gkH/RAx4QXE/HRSaclttQvu6Et2 K5AbZftkoOMulHPx+jDMVJrmt8xWjtQwM9sII+FKxWbdJW1A6zOzkTj6acbO8ues LCUS/lnBymh+0WSgKz4ooSjs+o0aa1hI89rECOVyY6w65ZxLtfAa5/hkCwLIIX7y 111S714KfHPlL9T2CoQhJzmvCQW3w0xf68O6XVuIdAgpPLSs5gMWFux6ICOxJ1AB B0PuCGs83WNjpzBD+K3SzmMo0uQSBdk/STF1wGeOEUwF/VCJG7CcEmXwIbv8pqr2 /z2GIYiopU/ra0HehGNYpEG42SMTATdRy1usboyJLLt6zIlGE+7RLaJWipUA34Y= =aXpl -END PGP SIGNATURE- php-7.0.0beta3.tar.xz SHA256 hash: f3cf545dcf74f4727f378dcfa206d7b3c78aafca3ccf68109d7302733fb957b4 PGP signature: -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAABAgAGBQJVwdsmAAoJELyqMOqcDVdjxrQIALWA12QTYzCQPVo7yMWWfKjR CiVxGVgH1wicK6pz5h3hWvLAct8812WT0FjBRPKojj4T4nPlJjAoxCl8j2YPHmee v6/TF3UrmXVjpSMvdxEYfc+bLmvVvRt1iiwrcus2tIeKy2vWT/mZckKo5GN5yS2B CzxpM4uLDbdzKunUkAl8mpmOjGWAXNILkt6LTPa+nkMA8+nTpPwsyN5McADxW4Nb DZnUBmPibn2oz0QRV8MQhPIvgjcjwgzVQYYtTzSgPNmV1g3hOzQhjBdiCXvAU7Ql wWm63/DYbOBkwF2lhzjek6as0+YzfLKBboFI+myFrbaOEAPdRHTIKhitEEdQ3GM= =jw/9 -END PGP SIGNATURE- Regards, Kalle Sommer Nielsen, Anatol Belski and Ferenc Kovacs -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
