[PHP-DEV] Re: Throwable::addSuppressed()
On 07/31/2015 08:25 PM, Markus Malkusch wrote: Stephen Coakley: Interesting thought, but how is this different than including a previous throwable and throwing a new, more descriptive exception? } catch (Exception $e1) { try { $resource-close(); } catch (ResourceException $e2) { // Pass in $e2 as the previous exception in the chain. throw new ResourceFailedException($e1-getMessage(), $e1-getCode(), $e2); } } Sorry, in my previous mail I actually forgot to answer your question. What you suggest is a common pattern to work around the lack of Throwable::addSupressed(). There are two big differences: 1) There's no causality between e1 and e2. You pass the message from e1 to the newly created ResourceFailedException which is caused by e2. Actually the causality here is correct, as ResourceFailedException is caused by e2. But what I understand is that ResourceFailedException should substitute e1 and e1 is not caused by e2. Just imagine e1 has the message insufficient funds. Then the user gets an exception which says insufficient funds is caused by could not rollback transaction. 2) You are loosing one stack trace (in this case from e1). For me the stack trace is the most important information in the exception. In this use case there are two stack traces and I want to have both of them. And then again this is just a simple example scenario with one resource. There might exist n resources. Markus Malkusch That makes sense -- I can see the uses for that. I don't mean to play devil's advocate, but is it worth sacrificing the immutability of exceptions for an addSupressed() method? Other than that, I think I would be for such an addition. -- Stephen Coakley -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
Hi! week or two and I serialize exceptions (excluding stack trace arguments) to send them back to the calling process to aid in debugging process failures. But then you don't need to serialize Exception. You need to send the text representation of Exception, for humans to look at, not the live Exception object. Sending the actual object would be next to impossible anyway (i.e. how you send over the live DB connection on DB query exception?). But the text representation includes the exception message, code, and the stack trace. That's pretty much the whole thing. But you are right, I don't want (and couldn't transfer) the live objects related to the stack trace, if that's what you're referring to. That's why I would omit the arguments in each stack trace item during serialization, because who knows what it could be. I just think that serializing exceptions is a buggy feature right now, and we should fix the feature instead of throwing it out. -- Stephen Coakley -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
On 1 August 2015 00:36:58 BST, Stanislav Malyshev smalys...@gmail.com wrote: Hi! As I have pointed out several times, it is only the 'args' section of the backtrace that ever contains unserializable items. The solution previous could too. In fact, right now, since you can unserialize exceptions, previous can contain literally anything and so can any other members. Also, user code can modify any protected properties too. By that logic, *no* object should be Serializable, because attempting to do so might recurse to a property that can't be. That doesn't make any sense; what we're talking about here are the native properties of a standard exception, not random data stuffed into the data by a user. DEBUG_BACKTRACE_IGNORE_ARGS in a debug_backtrace() call. IIRC the object of called methods is already excluded (equivalent to masking out DEBUG_PROVIDE_OBJECT) so what's left is all strings. I'm not sure how you arrived at the conclusion that all arguments in backtrace are strings. Arguments can be of any type. When printed, they are converted to strings, but they are not strings when stored. I'll have to recheck when I have more time, and something better than a phone to type on, but from memory, the backtrace which can be retrieved from an exception includes the same information as debug_backtrace(false), that is: - function: string - line: integer - file: string - class: string - type: string - args: array Of these 6 items, it is only 'args' that can contain an object of any kind, so without this item, the data would be serializable. This would be equivalent to debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS). If any of these items (other than args) are stored in memory in a different form (which doesn't seem likely to me), that form is completely inaccessible to the user, so converting to string during serialization would effectively be lossless. (Or, pre-converting would have zero BC break.) Similarly, if additional details are stored, those details are inaccessible, so removing them has no impact on any existing (userland) code. Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Sat, Aug 1, 2015 at 2:00 AM, Scott Arciszewski sc...@paragonie.com wrote: On Fri, Jul 31, 2015 at 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? -- Ferenc Kovács @Tyr43l - http://tyrael.hu Your vote is for apps to be insecure by default. my vote is pragmatic. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Nobody's going to do that though. The end result is going to be less security because of a cargo cult devotion to consistency. if you want to change the status quo you have to show a decent case how this is different than any other crypto related function. you can't because it isn't. This should be secure by default. The most secure way for an RNG to fail is to interrupt the application. This means: * E_ERROR * throw new Exception (or a subclass) * throw new Error (or a subclass) for some applications this is the best way, for some other applications it's none, because they have another source of entropy, or because they want to retry or fail gracefully. fatal errors should be used when there are no way to continue the execution, for this function it isn't always the case. Exceptions and Errors have the advantage that a developer who wants to go out of their way to handle them can simply do this: function randomPassword($length, $alphabet = 'abcdefghijklmnopqrstuvwxyz') { $sizeOfAlphabetMinusOne = strlen($alphabet) - 1; try { for ($i = 0; $i $length; ++$i) { $password .= $alphabet[random_int(0, $sizeOfAlphabetMinusOne)]; } } catch (Error $e) { return $this-framework-stylizedErrorMessage(RNG failure message here); } return $password; } Care to guess what returning false will do for $password? I don't have to guess, it is clear, I also understand where are you coming from, but I think your suggestion is too strict and it is fairly trivial to show similar misuse for any feature yet we have to draw a line somewhere as we can't save the developer from him/herself always. Any cryptography-related implementation needs to fail closed, not fail open. yet every entropy source function used by random_bytes() returns failure on error instead of calling exit. developer application library your library shouldn't guess about the intentions of the applications, nor should the application guess about the intentions of the developer. you create clean contracts and educate your developers to use those properly. By raising E_WARNING and returning false, you are placing an extra responsibility on the developer. not extra, but see above. Or as Daniel J. Bernstein would put it, YOU ARE BLAMING THE IMPLEMENTOR. Ask any competent application security expert, they'll back me up. Don't enforce insecure defaults just because it's more consistent. https://en.wikipedia.org/wiki/No_true_Scotsman Consistency is important, sure, but security is MORE important. clear contract is more important and having consistency saves the developer from the surprises which can be a good source of bugs. Also, death to libmcrypt:
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Ferenc, On Jul 31, 2015 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? Strong -1 on this. The consistency point is something to consider, but there are three major differences between password_* + mcrypt_* and this case. First, the majority of mcrypt and password functions are single use. Meaning they are called once and not inside of loops. Random_int on the other hand will be used in loops and in direct combination with other functions. Second, the target audience is different. Mcrypt targets those that know something about what they are doing. The api is designed that way. Don't get me wrong, a lot of users have no idea how to use the APIs properly. But the api is designed such that you need to know it well to use it effectively. This is a massive fundamental problem with how crypto code was implemented in php. Things have been changing lately though. Apis are being designed with end users in mind. Password_hash was one of these, but others are on their way as well. In general, the difference is putting the onus of correct usage on the designer rather than the implementor. Make it harder to screw up than to do it safe. Third and lastly, they were built in different times. Had exceptions been acceptable when I wrote the password functions, believe me I would have used them. But they weren't just not an option, they were strictly verboten. Today things are very different with 7. IMHO an exception is the right way to error here. The problems Scott raises are too big and have too much potential impact to just leave to the implementors. Instead, let's follow the trend of not handicapping what's possible. But instead making it harder to do something wrong than to do it right. After all, we are not discussing whether an error is appropriate here. We are discussing whether to fail in a way that forces the user to admit there was failure, or fail in a way they they can miss (leading to insecurity)... My $0.02 at least Anthony -- Ferenc Kovács @Tyr43l - http://tyrael.hu
[PHP-DEV] Re: Throwable::addSuppressed()
Stephen Coakley: Interesting thought, but how is this different than including a previous throwable and throwing a new, more descriptive exception? } catch (Exception $e1) { try { $resource-close(); } catch (ResourceException $e2) { // Pass in $e2 as the previous exception in the chain. throw new ResourceFailedException($e1-getMessage(), $e1-getCode(), $e2); } } Sorry, in my previous mail I actually forgot to answer your question. What you suggest is a common pattern to work around the lack of Throwable::addSupressed(). There are two big differences: 1) There's no causality between e1 and e2. You pass the message from e1 to the newly created ResourceFailedException which is caused by e2. Actually the causality here is correct, as ResourceFailedException is caused by e2. But what I understand is that ResourceFailedException should substitute e1 and e1 is not caused by e2. Just imagine e1 has the message insufficient funds. Then the user gets an exception which says insufficient funds is caused by could not rollback transaction. 2) You are loosing one stack trace (in this case from e1). For me the stack trace is the most important information in the exception. In this use case there are two stack traces and I want to have both of them. And then again this is just a simple example scenario with one resource. There might exist n resources. Markus Malkusch -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Fri, Jul 31, 2015 at 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? -- Ferenc Kovács @Tyr43l - http://tyrael.hu Your vote is for apps to be insecure by default. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Nobody's going to do that though. The end result is going to be less security because of a cargo cult devotion to consistency. This should be secure by default. The most secure way for an RNG to fail is to interrupt the application. This means: * E_ERROR * throw new Exception (or a subclass) * throw new Error (or a subclass) Exceptions and Errors have the advantage that a developer who wants to go out of their way to handle them can simply do this: function randomPassword($length, $alphabet = 'abcdefghijklmnopqrstuvwxyz') { $sizeOfAlphabetMinusOne = strlen($alphabet) - 1; try { for ($i = 0; $i $length; ++$i) { $password .= $alphabet[random_int(0, $sizeOfAlphabetMinusOne)]; } } catch (Error $e) { return $this-framework-stylizedErrorMessage(RNG failure message here); } return $password; } Care to guess what returning false will do for $password? Any cryptography-related implementation needs to fail closed, not fail open. By raising E_WARNING and returning false, you are placing an extra responsibility on the developer. Or as Daniel J. Bernstein would put it, YOU ARE BLAMING THE IMPLEMENTOR. Ask any competent application security expert, they'll back me up. Don't enforce insecure defaults just because it's more consistent. Consistency is important, sure, but security is MORE important. Also, death to libmcrypt: https://paragonie.com/blog/2015/05/if-you-re-typing-word-mcrypt-into-your-code-you-re-doing-it-wrong 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
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Fri, Jul 31, 2015 at 8:23 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Sat, Aug 1, 2015 at 2:00 AM, Scott Arciszewski sc...@paragonie.com wrote: On Fri, Jul 31, 2015 at 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? -- Ferenc Kovács @Tyr43l - http://tyrael.hu Your vote is for apps to be insecure by default. my vote is pragmatic. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Nobody's going to do that though. The end result is going to be less security because of a cargo cult devotion to consistency. if you want to change the status quo you have to show a decent case how this is different than any other crypto related function. you can't because it isn't. This should be secure by default. The most secure way for an RNG to fail is to interrupt the application. This means: * E_ERROR * throw new Exception (or a subclass) * throw new Error (or a subclass) for some applications this is the best way, for some other applications it's none, because they have another source of entropy, or because they want to retry or fail gracefully. fatal errors should be used when there are no way to continue the execution, for this function it isn't always the case. Exceptions and Errors have the advantage that a developer who wants to go out of their way to handle them can simply do this: function randomPassword($length, $alphabet = 'abcdefghijklmnopqrstuvwxyz') { $sizeOfAlphabetMinusOne = strlen($alphabet) - 1; try { for ($i = 0; $i $length; ++$i) { $password .= $alphabet[random_int(0, $sizeOfAlphabetMinusOne)]; } } catch (Error $e) { return $this-framework-stylizedErrorMessage(RNG failure message here); } return $password; } Care to guess what returning false will do for $password? I don't have to guess, it is clear, I also understand where are you coming from, but I think your suggestion is too strict and it is fairly trivial to show similar misuse for any feature yet we have to draw a line somewhere as we can't save the developer from him/herself always. Any cryptography-related implementation needs to fail closed, not fail open. yet every entropy source function used by random_bytes() returns failure on error instead of calling exit. developer application library your library shouldn't guess about the intentions of the applications, nor should the application guess about the intentions of the developer. you create clean contracts and educate your developers to use those properly. By raising E_WARNING and returning false, you are placing an extra responsibility on the developer. not extra, but see above. Or as Daniel J. Bernstein would put it, YOU ARE BLAMING THE IMPLEMENTOR. Ask any competent application security expert, they'll back me up. Don't enforce insecure defaults just because it's more consistent. https://en.wikipedia.org/wiki/No_true_Scotsman Consistency is important, sure, but security is MORE important. clear contract is more important and having consistency saves
[PHP-DEV] Exposing object handles to userland
Hi people. I've been pinged many times to add a new spl_object_id() function to PHP, that would return the internal object handle of an object. Today, spl_object_hash() partially allows that, but adds many randomness to the result, which is not very cool to use later (why does it even add randomness ?). There has been topics about this subject. For example, at http://marc.info/?l=php-internalsm=141814350920452w=2 Beeing able to get the object handle back in PHP userland would ease many tools, mainly debug-oriented tools. I know PHPUnit, Symfony and many big projects today make use of spl_object_hash() to identify objects. I also know people that print_r($an_object) and parse the output just to extract the object handle from there... Crazy isn't it ? Why couldn't we help those people by simply adding a new function that does the job ? Thoughts ? Julien.Pauli
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 31 Jul 2015, at 15:00, Joe Watkins pthre...@pthreads.org wrote: Even the best implementation need only have a single hole in it and everything falls apart, one extension not doing something properly, or one mistake in the implementation ... it's not hard to imagine these things happening, right ? Hi Joe, I've just replied to Matt and your email came in as I sent... so a bit of a change there. But what I was proposing was that PHP simply checks the way in which the variables (strings) are being passed around... if something from $_GET is passed into mysqli_query without being escaped (as a parameter or as a quoted escaped string), it will still continue, but a note will be written to the log (or on the page if display_errors is on). The same would happen when you echo a $_GET variable :-) So in 3015, if the taint checking isn't configured properly (switched on)... it wouldn't change the execution, it's just not checking things (oh well), it should still have been coded properly to begin with. So the way in which PHP executes is not effected, it's just picking up the oh, you probably shouldn't be doing this. Now Matt's suggestion allowed things to be actively blocked... I'm not sure I'd use that, but it might work for some (I'd just be happy having a log to check, in the same way that I use the logs for undefined variables, script timeouts, etc). And yes, you won't get a perfect system (there are some edge cases)... but keeping it as simple as possible (hence why I want to take a slightly different approach to the 2008 RFC), it should pick up the most common mistakes. But please do talk about security, you may be wrong, but thats fine, I'm sure someone will be able to correct you... I personally feel that too many people try to ignore security, and that's why we keep having so many problems (that said, performance and accessibility also need some attention, but that's more for the developers creating the websites, rather than PHP internals). Craig On 31 Jul 2015, at 15:00, Joe Watkins pthre...@pthreads.org wrote: Morning Craig, I think Pierre and I object to the concept, regardless of the intricacies of any particular implementation. Even the best implementation need only have a single hole in it and everything falls apart, one extension not doing something properly, or one mistake in the implementation ... it's not hard to imagine these things happening, right ? When I made reference to safe_mode, it wasn't because I misunderstood the concept, I know you're not introducing another safe mode, but introducing a INI, or build, or configuration option that says we got this is doomed to fail whatever. I don't like to talk about security, it's not something I feel qualified to talk about really, but I can observe that catch-all security features have already been proven to fail ... It's the year 3015, we're all running PHP442, our taint implementation is flawless, every major framework (which we have a futurish word for) relies on tainting and thinks SQLi is no longer a thing to worry about, because we got this. A junior sysadmin, on mars, is charged with upgrading PHP on the server that is responsible for crop irrigation on the terra-formed world, and forgets to configure taint properly. The upgrade goes live, renegade ruby fans hack the system disrupting food production, leading to the eventual death of everything. It's Friday afternoon, and that was a bit of fun ... I hope I lifted your mood ... push on, I can be wrong, Pierre can be wrong, we can all be wrong ... Cheers Joe On Fri, Jul 31, 2015 at 11:40 AM, Craig Francis cr...@craigfrancis.co.uk wrote: On 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote: Even if some of those people replying haven't read or don't understand what you are suggesting, it is not a good tactic to assume that and reply with read the RFC. Hi Joe, Sorry about yesterday, I have done as you have said, and I have read those responses again, but unfortunately I still feel that I was right in my assumptions (notes below, maybe some interesting additions?). In general I have been getting very frustrated that no-one seems to really care about security (and to be fair, it has been annoying me for far too many years). Keeping in mind that I work with other developers who routinely keep introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying things like switching the CSP off, because they copy/paste some hideous/insecure JS, and can't be bothered to work out why the eval function isn't working. So maybe I should start a new thread, without Matt's subject (btw Matt, I really appreciate what you are trying todo, I disagree with the blocking element, and I think we can also address more than just SQL injection vulnerabilities)... maybe something like I've found this one weird trick
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 Joe! To be clear, this feature is not an alternative to coding SQL securely with parameterized queries. Indeed, it's entire purpose is to help developers move dangerously coded queries (i.e. ones that are not parameterized and are dynamically constructed using parameters that attackers might control, like $_GET[], or the output of other database queries) to ones that are clearly and unambiguously secure, by parameterizing their queries. This feature is not a replacement for the security guidance of parameterize all your queries; but it does allow webapplication developers who follow this guidance to enforce it at the language level to ensure that they don't let a single SQL-injectable query through the net. Enabling this feature encourages developers to write code that is secure even when this feature is disabled. Hope that clarifies! Matt On 31 July 2015 at 15:00, Joe Watkins pthre...@pthreads.org wrote: Morning Craig, I think Pierre and I object to the concept, regardless of the intricacies of any particular implementation. Even the best implementation need only have a single hole in it and everything falls apart, one extension not doing something properly, or one mistake in the implementation ... it's not hard to imagine these things happening, right ? When I made reference to safe_mode, it wasn't because I misunderstood the concept, I know you're not introducing another safe mode, but introducing a INI, or build, or configuration option that says we got this is doomed to fail whatever. I don't like to talk about security, it's not something I feel qualified to talk about really, but I can observe that catch-all security features have already been proven to fail ... It's the year 3015, we're all running PHP442, our taint implementation is flawless, every major framework (which we have a futurish word for) relies on tainting and thinks SQLi is no longer a thing to worry about, because we got this. A junior sysadmin, on mars, is charged with upgrading PHP on the server that is responsible for crop irrigation on the terra-formed world, and forgets to configure taint properly. The upgrade goes live, renegade ruby fans hack the system disrupting food production, leading to the eventual death of everything. It's Friday afternoon, and that was a bit of fun ... I hope I lifted your mood ... push on, I can be wrong, Pierre can be wrong, we can all be wrong ... Cheers Joe On Fri, Jul 31, 2015 at 11:40 AM, Craig Francis cr...@craigfrancis.co.uk wrote: On 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote: Even if some of those people replying haven't read or don't understand what you are suggesting, it is not a good tactic to assume that and reply with read the RFC. Hi Joe, Sorry about yesterday, I have done as you have said, and I have read those responses again, but unfortunately I still feel that I was right in my assumptions (notes below, maybe some interesting additions?). In general I have been getting very frustrated that no-one seems to really care about security (and to be fair, it has been annoying me for far too many years). Keeping in mind that I work with other developers who routinely keep introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying things like switching the CSP off, because they copy/paste some hideous/insecure JS, and can't be bothered to work out why the eval function isn't working. So maybe I should start a new thread, without Matt's subject (btw Matt, I really appreciate what you are trying todo, I disagree with the blocking element, and I think we can also address more than just SQL injection vulnerabilities)... maybe something like I've found this one weird trick that will fix every security issue... sorry, I hate that kind of approach, but if it gets a response, maybe its worth it :-) Craig -- http://news.php.net/php.internals/87346 From: Matt Tait Reply: N/A Original suggestion. -- http://news.php.net/php.internals/87348 From: Rowan Collins Reply: Matt Tait Suggestion to Matt to look for previous RFCs. -- http://news.php.net/php.internals/87350 From: Christoph Becker Reply: Matt Tait Points out the existing Taint RFC from 2008, by Wietse Venema. -- http://news.php.net/php.internals/87355 From: Pierre Joye Reply: Matt Tait Pointing out that there is more than SQLi (true), and it might be an impossible task (I disagree, as explained later). -- http://news.php.net/php.internals/87361 From: Thomas Bley Reply: Matt Tait Suggesting the use of bound parameters / prepared statements, which I agree is how it should be coded by the website developers, but I think the
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
This may have been true at one point in time, but my own experience and the statistics collected by Dan Kaminsky of White Hat Security indicates that Cross-Site Scripting vulnerabilities are much more prevalent in 2015 than SQL Injection, especially in business applications. If Google has information that indicates that SQLi is still more prevalent than XSS, I'd love to see this data. Thanks Scott! You are absolutely correct. SQL injection is a solved problem using parameterized SQL. In fact, parameterizing queries to avoid SQL injection is so uncontroversially accepted security advice, that this RFC *enforces* the advice at the language level (unless the website administrator explicitly disables the protection across the website via PHP.INI). Examples of SQL queries that *would* and *would not *be blocked by this tool are given in the RFC. In your case, ORDER BY {$column} ASC would not be blocked by this tool if $column came from within the PHP application, but would be blocked if it came from, say, $_GET[column]. Sadly, although SQL injection is a solved problem by using prepared statements, not everyone follows the advice. Prior to working at Google, I worked with a company called iSEC Partners who do application review for various companies. From my experience, between a third and a half of all the often-huge PHP websites I security-reviewed were vulnerable to SQL injection somewhere. It wasn't uncommon for the company to have ~5000 or so SQL queries in their PHP code, only 2-3 of which were vulnerable to injection. But attackers only need one to ruin your company and steal your users' data. To take apart some of the other valid comments on this thread: == SQL injection is dead == This is objectively not the case https://www.exploit-db.com/search/?action=searchdescription=SQL+injection == Won't this break every website? == No. In its initial release, this feature will be released in disabled mode. Website owners with heightened security requirements will be able to upgrade this to logging or blocking mode. This means this feature will have zero impact on websites that do not explicitly enable it. == We should be looking at XSS instead == Solving SQL injection with parameterized SQL queries is uncontroversial and universally accepted security advice. Lots of different (some working, some not) advice for solving XSS exist, but this makes it harder to standardize one of those ideas into the language without significant controversy. For example, one website may choose to store HTML content in their database. This should be printed out to HTML verbatim. Other websites may accidentally store HTML content in their database, leading to stored-XSS. The language will not be able to distinguish these two use-cases. In contrast, taking content out of a database and gluing it into a SQL statement is /always/ an error, as it can lead to second-order and blind-SQL injection vulnerabilities. For this reason, this RFC will initially only be looking at securing websites against SQL-injections, although perhaps there is scope to also look at XSS-injection vulnerabilities in a different RFC using similar tools at a later date. == This breaks applications that use OOP or other complicated ways of sending data to a database == This is not the case. This RFC doesn't use taint analysis to find SQL injections; it uses reverse-taint analysis to avoid blocking provably safe non-parameterized queries (so we don't block queries that are safely built dynamically out of static strings and variables). Many examples of this are given in the RFC. As you can see from this proof-of-concept ( http://phpoops.cloudapp.net/oops.php?action=maindbg_sqllimit=4%20uhoh), all of the SQL statements are unparameterized, but only one unparameterized SQL statement that includes attacker-controllable data is blocked, which, as it turns out, is the only query vulnerable to SQL injection. Dynamically construct SQL statements out of provably safe components are not detected as a SQL-injectable error by this RFC. == SQL injection is already solved! Why are you re-solving it? == This RFC doesn't solve SQL injection in a new way. It blocks dynamic SQL queries that are not parameterized by default, but uses reverse-taint analysis to reduce false positives where developers are building SQL query string out of other constant strings (such as config variables, table names, column names etc). This feature only takes action when tainted data is concatenated into a SQL query string and that query string is issued against the database, and even then, only if PHP.INI has been configured to do so. == We should log, rather than block exploit attempts == The feature has three modes, which is configurable via PHP.INI: Safe mode causes the query to be aborted, and a SecurityError exception thrown. Logging mode causes the query to be run, and an E_WARNING raised Ignore more causes the query to be run and no other action to be taken. The default mode
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Morning Craig, I think Pierre and I object to the concept, regardless of the intricacies of any particular implementation. Even the best implementation need only have a single hole in it and everything falls apart, one extension not doing something properly, or one mistake in the implementation ... it's not hard to imagine these things happening, right ? When I made reference to safe_mode, it wasn't because I misunderstood the concept, I know you're not introducing another safe mode, but introducing a INI, or build, or configuration option that says we got this is doomed to fail whatever. I don't like to talk about security, it's not something I feel qualified to talk about really, but I can observe that catch-all security features have already been proven to fail ... It's the year 3015, we're all running PHP442, our taint implementation is flawless, every major framework (which we have a futurish word for) relies on tainting and thinks SQLi is no longer a thing to worry about, because we got this. A junior sysadmin, on mars, is charged with upgrading PHP on the server that is responsible for crop irrigation on the terra-formed world, and forgets to configure taint properly. The upgrade goes live, renegade ruby fans hack the system disrupting food production, leading to the eventual death of everything. It's Friday afternoon, and that was a bit of fun ... I hope I lifted your mood ... push on, I can be wrong, Pierre can be wrong, we can all be wrong ... Cheers Joe On Fri, Jul 31, 2015 at 11:40 AM, Craig Francis cr...@craigfrancis.co.uk wrote: On 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote: Even if some of those people replying haven't read or don't understand what you are suggesting, it is not a good tactic to assume that and reply with read the RFC. Hi Joe, Sorry about yesterday, I have done as you have said, and I have read those responses again, but unfortunately I still feel that I was right in my assumptions (notes below, maybe some interesting additions?). In general I have been getting very frustrated that no-one seems to really care about security (and to be fair, it has been annoying me for far too many years). Keeping in mind that I work with other developers who routinely keep introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying things like switching the CSP off, because they copy/paste some hideous/insecure JS, and can't be bothered to work out why the eval function isn't working. So maybe I should start a new thread, without Matt's subject (btw Matt, I really appreciate what you are trying todo, I disagree with the blocking element, and I think we can also address more than just SQL injection vulnerabilities)... maybe something like I've found this one weird trick that will fix every security issue... sorry, I hate that kind of approach, but if it gets a response, maybe its worth it :-) Craig -- http://news.php.net/php.internals/87346 From: Matt Tait Reply: N/A Original suggestion. -- http://news.php.net/php.internals/87348 From: Rowan Collins Reply: Matt Tait Suggestion to Matt to look for previous RFCs. -- http://news.php.net/php.internals/87350 From: Christoph Becker Reply: Matt Tait Points out the existing Taint RFC from 2008, by Wietse Venema. -- http://news.php.net/php.internals/87355 From: Pierre Joye Reply: Matt Tait Pointing out that there is more than SQLi (true), and it might be an impossible task (I disagree, as explained later). -- http://news.php.net/php.internals/87361 From: Thomas Bley Reply: Matt Tait Suggesting the use of bound parameters / prepared statements, which I agree is how it should be coded by the website developers, but I think the PHP language itself can help identify when this has not been done (just by raising a notice, not blocking anything). Also Thomas points out that static code analysis could identify these issues, but these are far from perfect, and PHP is in a much better position to be doing this... and has the advantage of being available to everyone. Just to note, I've played with a couple of static code analysers which cost in the region of £19,000 per year, and they still don't find the same number of escaping issues that my suggestion can find (they do look at other issues, so don't get rid of them, but this one thing that can be done better with the programming language itself). -- http://news.php.net/php.internals/87363 From: Lester Caine Reply: Matt Tait Short answer saying you should not use mysql... which I think is a bit short sighted (on the assumption that mysqli is a similar interface for most
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, I take it all back... I've been so sure of my own suggestion, I've not really given your RFC a proper read... and I think this could work. The main difference I had was the ability to support to support the escaping functions (see pg_escape_literal for PostgreSQL, or htmlentities for html output)... and your solution of not supporting any of them keeps the whole thing very simple. Personally I would still like to check html encoding, which your solution won't do... but I realise that is quite tricky, and won't be a perfect solution: $url = 'javascript:alert(document.cookies)'; echo 'a href=' . htmlentities($url) . ''; But may I request 2 changes: 1) Rather than calling the zend_string property sql-safe constant, could it simply be string literal, as in, the variable has only been created by T_STRING's... as this could be used for other things, e.g. executing commands via exec_bind: http://news.php.net/php.internals/87361 2) Include a function that allows the PHP developers to check if the variable being passed is a string literal, as frameworks can check the state before performing an action, e.g. they could provide the exec_bind function: function exec_bind($cmd, $arguments) { if (!is_string_literal($cmd)) { throw new Exception('...'); } // using escapeshellarg for the $arguments } Craig On 31 Jul 2015, at 14:11, Matt Tait matt.t...@gmail.com wrote: This may have been true at one point in time, but my own experience and the statistics collected by Dan Kaminsky of White Hat Security indicates that Cross-Site Scripting vulnerabilities are much more prevalent in 2015 than SQL Injection, especially in business applications. If Google has information that indicates that SQLi is still more prevalent than XSS, I'd love to see this data. Thanks Scott! You are absolutely correct. SQL injection is a solved problem using parameterized SQL. In fact, parameterizing queries to avoid SQL injection is so uncontroversially accepted security advice, that this RFC *enforces* the advice at the language level (unless the website administrator explicitly disables the protection across the website via PHP.INI). Examples of SQL queries that *would* and *would not *be blocked by this tool are given in the RFC. In your case, ORDER BY {$column} ASC would not be blocked by this tool if $column came from within the PHP application, but would be blocked if it came from, say, $_GET[column]. Sadly, although SQL injection is a solved problem by using prepared statements, not everyone follows the advice. Prior to working at Google, I worked with a company called iSEC Partners who do application review for various companies. From my experience, between a third and a half of all the often-huge PHP websites I security-reviewed were vulnerable to SQL injection somewhere. It wasn't uncommon for the company to have ~5000 or so SQL queries in their PHP code, only 2-3 of which were vulnerable to injection. But attackers only need one to ruin your company and steal your users' data. To take apart some of the other valid comments on this thread: == SQL injection is dead == This is objectively not the case https://www.exploit-db.com/search/?action=searchdescription=SQL+injection == Won't this break every website? == No. In its initial release, this feature will be released in disabled mode. Website owners with heightened security requirements will be able to upgrade this to logging or blocking mode. This means this feature will have zero impact on websites that do not explicitly enable it. == We should be looking at XSS instead == Solving SQL injection with parameterized SQL queries is uncontroversial and universally accepted security advice. Lots of different (some working, some not) advice for solving XSS exist, but this makes it harder to standardize one of those ideas into the language without significant controversy. For example, one website may choose to store HTML content in their database. This should be printed out to HTML verbatim. Other websites may accidentally store HTML content in their database, leading to stored-XSS. The language will not be able to distinguish these two use-cases. In contrast, taking content out of a database and gluing it into a SQL statement is /always/ an error, as it can lead to second-order and blind-SQL injection vulnerabilities. For this reason, this RFC will initially only be looking at securing websites against SQL-injections, although perhaps there is scope to also look at XSS-injection vulnerabilities in a different RFC using similar tools at a later date. == This breaks applications that use OOP or other complicated ways of sending data to a database == This is not the case. This RFC doesn't use taint analysis to find SQL injections; it uses reverse-taint analysis to avoid blocking provably safe non-parameterized queries (so we don't block
RE: [PHP-DEV] Introduction and some opcache SSE related stuff
Running php-cgi -T1 on WordPress4.1/index.php I see ~1% performance increase for the new version of fast_memcpy() compared with the generic memcpy(). Same result using a full load test with http_load on a Haswell EP 18 cores. 1% is really big improvement. I'll able to check this only on next week (when back from vacation). Well, he talks like he was comparing to *generic* memcpy(), so...? But not sure how that would have been accomplished. BTW guys, I was wondering before why fast_memcpy() only in this opcache area? For the prefetch and/or cache pollution reasons? Just because, in this place we may copy big blocks, and we also may align them properly, to use compact and fast Inlined code. Yeah... in fact all my numbers are against the current fast_memcpy() implementation, not against generic memcpy(). Sorry for the misleading information... :-/. I was playing in my corner with some SSE4.2 experiments and I wasn’t aware that SSE2 is enabled by default without any need of compiler switch. Coming back to the issue and trying to answer also to laruence’s request for more numbers: I am running php-cgi -T1 on a Haswell having 45MB L3 cache: The improvement is visible for scenarios where the amount of data loaded via opcache is significant while the real execution time is not so big; this is the case of real life scenarios: - WordPress 4.1 MediaWiki 1.24: ~1% performance increase - Drupal 7.36: ~0.6% performance increase - The improvement is not visible on synthetic benchmarks (mandelbrot, micro_bench, …) which load a small amount of bytecode and are computing intensive. The explanation stays in data cache misses. I did a deeper analysis on Wordpress 4.1 using perf tool: - _mm_stream based implementation: ~3x10^-4 misses/instruction = 1.023 instructions/cycle - _mm_store based implementation: ~9x10^-6 misses/instruction (33x less) = 1.035 instructions/cycle So the overall performance gain is fully explained by the increase of instructions/cycle due to lower cache misses; copying the opcache data is a kind of software prefetcher for further execution. This phenomenon is most visible on processors with big caches. If I go to a lower L3 cache size (45MB - 6.75MB) 1% WP gain became 0.6% gain (as the cache capability to keep prefetched opcahe data without polluting the execution path become smaller). Coming back to generic memcpy(), the fast_memcpy() implementation seems to be a very little bit smaller in terms of executed instructions (hard to measure the real IR data due to run to run variations). Doing a couple of measurements for absorbing run to run effect I see ~0.2% perfo increase in favor of fast_memcpy w/ mm_store; it is the same increase I see in the implementation w/ SW prefetchers compared with the case of no SW prefetch in place. So the gain we see might be explained by the fact that memcpy() do not use SW prefetching - just a guess... Kind Regards, Bogdan
RE: [PHP-DEV] Exposing object handles to userland
Hi, -Original Message- From: julienpa...@gmail.com [mailto:julienpa...@gmail.com] On Behalf Of Julien Pauli Sent: Friday, July 31, 2015 4:24 PM To: PHP Internals internals@lists.php.net Subject: [PHP-DEV] Exposing object handles to userland Hi people. I've been pinged many times to add a new spl_object_id() function to PHP, that would return the internal object handle of an object. Today, spl_object_hash() partially allows that, but adds many randomness to the result, which is not very cool to use later (why does it even add randomness ?). There has been topics about this subject. For example, at http://marc.info/?l=php-internalsm=141814350920452w=2 Beeing able to get the object handle back in PHP userland would ease many tools, mainly debug-oriented tools. I know PHPUnit, Symfony and many big projects today make use of spl_object_hash() to identify objects. I also know people that print_r($an_object) and parse the output just to extract the object handle from there... Crazy isn't it ? Why couldn't we help those people by simply adding a new function that does the job ? Regarding the ID keywords, that reminds on https://docs.python.org/2/library/functions.html#id . While not everything is object in PHP, fe it could be interesting at least for the strings since we have interned strings. Maybe there were some wider range of useful information to provide in user land, not only related to objects. Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Exposing object handles to userland
I also know people that print_r($an_object) and parse the output just to extract the object handle from there... Crazy isn't it ? I plead guilty for doing this, but php let me no better choice for now ;) The attached patch removes the XOR hashing for the object handle (it's useless, the secret is trivially guessed after parsing the output of var_dump). It would be awesome if this patch could be applied for php 7.0! Cheers, Nicolas http://marc.info/?l=php-internalsm=141811755908008w=2 diff --git a/ext/spl/php_spl.c b/ext/spl/php_spl.c index e89caa2..0d8be97 100644 --- a/ext/spl/php_spl.c +++ b/ext/spl/php_spl.c @@ -755,22 +755,20 @@ PHP_FUNCTION(spl_object_hash) PHPAPI zend_string *php_spl_object_hash(zval *obj) /* {{{*/ { - intptr_t hash_handle, hash_handlers; + intptr_t hash_handlers; if (!SPL_G(hash_mask_init)) { if (!BG(mt_rand_is_seeded)) { php_mt_srand((uint32_t)GENERATE_SEED()); } - SPL_G(hash_mask_handle) = (intptr_t)(php_mt_rand() 1); SPL_G(hash_mask_handlers) = (intptr_t)(php_mt_rand() 1); SPL_G(hash_mask_init) = 1; } - hash_handle = SPL_G(hash_mask_handle)^(intptr_t)Z_OBJ_HANDLE_P(obj); hash_handlers = SPL_G(hash_mask_handlers)^(intptr_t)Z_OBJ_HT_P(obj); - return strpprintf(32, %016lx%016lx, hash_handle, hash_handlers); + return strpprintf(32, %016lx%016lx, (intptr_t)Z_OBJ_HANDLE_P(obj), hash_handlers); } /* }}} */ diff --git a/ext/spl/php_spl.h b/ext/spl/php_spl.h index 015ada4..b2450ca 100644 --- a/ext/spl/php_spl.h +++ b/ext/spl/php_spl.h @@ -62,7 +62,6 @@ PHP_MINFO_FUNCTION(spl); ZEND_BEGIN_MODULE_GLOBALS(spl) zend_string *autoload_extensions; HashTable *autoload_functions; - intptr_t hash_mask_handle; intptr_t hash_mask_handlers; int hash_mask_init; int autoload_running; diff --git a/ext/spl/tests/spl_005.phpt b/ext/spl/tests/spl_005.phpt index 219c791..c1bffe7 100644 --- a/ext/spl/tests/spl_005.phpt +++ b/ext/spl/tests/spl_005.phpt @@ -11,7 +11,7 @@ var_dump(spl_object_hash()); ===DONE=== ?php exit(0); ? --EXPECTF-- -string(32) %s +string(32) 000%r[1-9]%r%s Warning: spl_object_hash() expects parameter 1 to be object, integer given in %sspl_005.php on line %d NULL -- 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
On 28/07/15 18:33, Matt Tait wrote: What do you all think? There's obviously a bit more work to do; the PoC currently only covers mysqli_query, but I thought this stage is an interesting point to throw it open to comments before working to complete it. So who addresses all the other database drivers? Which is something other ''proposals' currently ignore as well. -- Lester Caine - G8HFL - Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Benchmark Results for PHP Master 2015-07-30
-Original Message- From: Christopher Jones [mailto:christopher.jo...@oracle.com] Sent: Friday, July 31, 2015 1:12 AM To: internals@lists.php.net Subject: Re: [PHP-DEV] Benchmark Results for PHP Master 2015-07-30 On 30/07/2015 11:12 pm, Niklas Keller wrote: 2015-07-30 14:42 GMT+02:00 Andone, Bogdan bogdan.and...@intel.com: -Original Message- From: Niklas Keller [mailto:m...@kelunik.com] Sent: Thursday, July 30, 2015 1:47 PM To: Pierre Joye Cc: lp_benchmark_robot; PHP internals; l...@lists.01.org Subject: Re: [PHP-DEV] Benchmark Results for PHP Master 2015-07-30 2015-07-30 11:57 GMT+02:00 Pierre Joye pierre@gmail.com: Hi, Does someone has a contact there? It would be nicer to have these results combined with what we pushed on qa.php.net as well. Cheers, Pierre Thought about that as well, results per mail aren't that useful, especially as they're badly formatted for me in GMail (no fixed font). A graph visualizing those numbers would be nice. Regards, Niklas Hi Guys, We are glad that our small ticking spam start to be observed :) ! We would like to offer valuable information to the community related to performance trends of the PHP project on Intel platforms based on daily builds and we are open for suggestions for making these results relevant. We chose to share our numbers as plain text mails for easily seeing the summary snapshots on discussion lists without the need of other clicks. Everybody agrees that plain text is ugly and, yes, you need to have fixed font in place for having the table formatted correctly. Let’s discuss a better way of doing; integration with qa.php.net is possible if we find the right interface for sharing data in an automated way. Normally l...@lists.01.org should be the official entry for feedbacks and requests but, unfortunately, it is not yet operational, so I will be your direct contact as I am part of the team which deploys this project. Kind regards, Bogdan Hi Bogdan, I think absolute numbers (instead of a %-change) would be better suited for visualizing performance over time. Regards, Niklas I agree on using absolute numbers. With percentages it is not immediately obvious whether the change was good or bad. Including the build options would be good. Chris -- http://twitter.com/ghrd -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php We can work on build option documentation. Most probably we will offer a link on the 01.org containing details on the environment configuration. Concerning absolute numbers I am afraid it will be difficult to publish them in this way. Our intention is to allow you to see progresses trend in the context of Zend PHP project only. We would like to avoid the temptation to use the absolute numbers for comparisons with other projects; they might be misleading and could drive to wrong conclusions if they are removed from the context. If the PHP baseline is relevant, the fact you see absolute numbers or percents should not make a real difference. We chose php-7.0.0beta1 in a somehow subjective way as they are no official PHP7 releases yet but we can take a PHP5.x release baseline if it looks more relevant. Kind Regards, Bogdan
Re: [PHP-DEV] Exposing object handles to userland
Julien, On Fri, Jul 31, 2015 at 10:23 AM, Julien Pauli jpa...@php.net wrote: Hi people. I've been pinged many times to add a new spl_object_id() function to PHP, that would return the internal object handle of an object. Today, spl_object_hash() partially allows that, but adds many randomness to the result, which is not very cool to use later (why does it even add randomness ?). There has been topics about this subject. For example, at http://marc.info/?l=php-internalsm=141814350920452w=2 Beeing able to get the object handle back in PHP userland would ease many tools, mainly debug-oriented tools. I know PHPUnit, Symfony and many big projects today make use of spl_object_hash() to identify objects. I also know people that print_r($an_object) and parse the output just to extract the object handle from there... Crazy isn't it ? Why couldn't we help those people by simply adding a new function that does the job ? Thoughts ? I'm not sure about the randomness to the handle, but the hash also includes the object handler pointer. So without the random, we'd be leaking information about the memory layout of the application. And for the record, I am cool with simply exposing the handle. Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Exposing object handles to userland
Hi Nicolas, Have you checked the impact of changing the existing function? Like https://github.com/sebastianbergmann/phpunit/search?utf8=%E2%9C%93 https://github.com/sebastianbergmann/phpunit/search?utf8=%E2%9C%93q=spl_object_hash q=spl_object_hash https://github.com/symfony/symfony/blob/master/src/Symfony/Component/VarDumper/Cloner/VarCloner.php https://github.com/horde/horde/blob/master/imp/lib/Factory/MimeViewer.php https://github.com/horde/horde/blob/master/framework/Support/lib/Horde/Support/Randomid.php https://github.com/WordPress/WordPress/blob/master/wp-includes/plugin.php Obviously there will be more. Seems some stuff even depends on the exact output. So it would be most likely a BC break. Now, I’d see this more like a feature request, as Julien was mentioning another function at the start. But IMHO opinions from people specializing on test/debug user land tools would give a better sight, so CC’ing. Regards Anatol From: nicolas.gre...@gmail.com [mailto:nicolas.gre...@gmail.com] On Behalf Of Nicolas Grekas Sent: Friday, July 31, 2015 4:53 PM To: Julien Pauli jpa...@php.net Cc: PHP Internals internals@lists.php.net Subject: Re: [PHP-DEV] Exposing object handles to userland I also know people that print_r($an_object) and parse the output just to extract the object handle from there... Crazy isn't it ? I plead guilty for doing this, but php let me no better choice for now ;) The attached patch removes the XOR hashing for the object handle (it's useless, the secret is trivially guessed after parsing the output of var_dump). It would be awesome if this patch could be applied for php 7.0! Cheers, Nicolas
Re: [PHP-DEV] Exposing object handles to userland
Have you checked the impact of changing the existing function? Yes I did, and this breaks absolutely nothing: the spl_object_hash output has exactly the same format (otherwise it's a bug). https://github.com/sebastianbergmann/phpunit/search?utf8=%E2%9C%93q=spl_object_hash https://github.com/symfony/symfony/blob/master/src/Symfony/Component/VarDumper/Cloner/VarCloner.php That's my code and the very place where I'd use this feature first :)
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 Thu, Jul 30, 2015 at 8:38 AM, Craig Francis cr...@craigfrancis.co.uk wrote: On 30 Jul 2015, at 16:26, Ronald Chmara rona...@gmail.com wrote: Perhaps I have missed something in this discussion I think you have... my email from a couple of weeks ago was ignored... so I replied to Matt's suggestion (which is similar, but different). Please, just spend a few minutes reading my suggestion, it has absolutely nothing todo with breaking applications: http://news.php.net/php.internals/87207 https://bugs.php.net/bug.php?id=69886 The RFC and bug report both make an erroneous assumption that unescaped GPC input is wrong. I was addressing some cases where it is the correct, intended, behavior. WRT breaking: Application stacks which go from zero E_NOTICE warnings, to hundreds or thousands of them a second or day, is (admittedly) poorly phrased as breaking. It is a possible side effect of the proposed solutiions. I have worked in very stingent environments where an unapproved E_NOTICE is considered a ship-stop break, but I did not explicitly explain that. Such environments would require re-writes of all SQL that throws an E_NOTICE, or a per-line exception review and approval process, or disabling/not enabling the checking. And yes, I do have a bypass_the_nerfing function (well, a function to say the variable has already been escaped)... but the idea is that it's ever so slightly harder to use than the related escaping functions, and rarely needed. Rarely is subjective at this point, a quanyifyable sampling of some kind could be more meaningful. (How rare?) Based on *my* purely anecdotal experience, in the last X years of using PHP I have have frequently encountered situations where passing through engine-unescaped text strings, to SQL, is desired for some reason, in nearly every environment. I mentioned one use case that I thought might be relevant to a large number of users, there are others. Off the top of my head, some use cases I have dealt with relevant to this discussion, that should be considered (even if they're discarded as trivial): 1. Administrator has a web application that is supposed to allow them access functionally equivalent to a direct connection to a database. 2. Overhead of using the engine escaping technique (setup connection(if not done yet), send text to escape at network speed, recieve escaped text at network speed) was considered too much of an additional performance hit. 3. Text needed to have a generic, user written, escaping library/function to handle multiple destinations (think 5 different data storage systems, all with different escaping rules, some without connection based escaping). 4. Text being supplied was coming from a pre-cleaned, trusted, source, even though the variable was GPC, (example: it was a GET string assembled by a batch job that was pulling from another database) I'm sure there are many more. Basing language decisions on personal perceptions of rarely and frequently is not a good practice, but ensuring that we are covering multiple use-cases is. Discussing the use cases doesn't mean I think the idea is without merit. -Ronabop
Re: [PHP-DEV] Exposing object handles to userland
Anthony's argument about exposing the mem layout is crucial, though. Yes it is! The patch I attached un-xors only the part for the object's handle. The memory pointer is kept xored.
RE: [PHP-DEV] Exposing object handles to userland
-Original Message- From: nicolas.gre...@gmail.com [mailto:nicolas.gre...@gmail.com] On Behalf Of Nicolas Grekas Sent: Friday, July 31, 2015 6:11 PM To: Anatol Belski anatol@belski.net Cc: Julien Pauli jpa...@php.net; PHP Internals internals@lists.php.net; Sebastian Bergmann sebast...@php.net; Ivan Enderlin@Hoa ivan.ender...@hoa-project.net; cont...@jubianchi.fr Subject: Re: [PHP-DEV] Exposing object handles to userland Have you checked the impact of changing the existing function? Yes I did, and this breaks absolutely nothing: the spl_object_hash output has exactly the same format (otherwise it's a bug). https://github.com/sebastianbergmann/phpunit/search?utf8=%E2%9C%93q= spl_object_hash https://github.com/symfony/symfony/blob/master/src/Symfony/Component/V arDumper/Cloner/VarCloner.php That's my code and the very place where I'd use this feature first :) Yeah, saw your name in the commits :) Anthony's argument about exposing the mem layout is crucial, though. Regards anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
On Tue, Jul 28, 2015 at 6:40 PM, Stanislav Malyshev smalys...@gmail.com wrote: Hi! -1 on this. If there is no technical problem with serializing the Exception class itself, it should be possible to serialize it. It can always happen that an object contains some not-serializable member, this is nothing specific to exceptions. I don't see the point of this change. The point is exactly that Exception contains non-serializable members that makes successfully serializing and restoring them impossible. You could get back something that looks like an Exception, but not the original one - namely, you can not store and restore backtraces. Personally I feel the restoring them impossible argument weak, consider that we allow stuff like serializing resources without even a notice. Based on my own experiences where I had to fix multiple apps when we introduced the unserializable Closure (mostly error logger and debugging tools) which got passed as argument in the backtrace I would prefer if we could remove that restriction. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
[PHP-DEV] Re: Exposing object handles to userland
On 07/31/2015 09:23 AM, Julien Pauli wrote: Hi people. I've been pinged many times to add a new spl_object_id() function to PHP, that would return the internal object handle of an object. Today, spl_object_hash() partially allows that, but adds many randomness to the result, which is not very cool to use later (why does it even add randomness ?). There has been topics about this subject. For example, at http://marc.info/?l=php-internalsm=141814350920452w=2 Beeing able to get the object handle back in PHP userland would ease many tools, mainly debug-oriented tools. I know PHPUnit, Symfony and many big projects today make use of spl_object_hash() to identify objects. I also know people that print_r($an_object) and parse the output just to extract the object handle from there... Crazy isn't it ? Why couldn't we help those people by simply adding a new function that does the job ? Thoughts ? Julien.Pauli I can think of several use cases why this might be useful, and not just for debugging-related code. It could be used for indexing some sort of complex object storage data structure (if you can't/won't use SplObjectStorage). I can think of a few libraries that us spl_object_hash() to do the same thing but doesn't work well in conjunction with forking (probably due to the randomness factor). +1 -- Stephen Coakley -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
On 07/28/2015 03:46 PM, Stanislav Malyshev wrote: Hi! This sort of change would be a major BC break for 8.x or similar. How is it a major BC break? You make it sound like serializing exceptions is something no application can do without. I have yet to see a single case where it's useful (yes, I've read the Symphony comment but I'm not sure why they're doing it and if it's indeed something that should be done and not an ugly hack like unserializing fake internal objects). I also don't see security implications, tbh. I don't want to discuss it in detail yet, but check out currently open or recently fixed security issues and see how many of them relate to serialized exceptions and consequences of that. -- Stas Malyshev smalys...@gmail.com Serializing exceptions can be useful in parallel code using multiple processes or threads. I have been working on a concurrency library for a week or two and I serialize exceptions (excluding stack trace arguments) to send them back to the calling process to aid in debugging process failures. I agree there aren't too many use cases, but there are a few. Of course, exceptions aren't *consistently* serializable, which is still a problem that should be resolved in some way. -- Stephen Coakley -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Exposing object handles to userland
Nicolas, On Fri, Jul 31, 2015 at 2:24 PM, Nicolas Grekas nicolas.grekas+...@gmail.com wrote: Anthony's argument about exposing the mem layout is crucial, though. Yes it is! The patch I attached un-xors only the part for the object's handle. The memory pointer is kept xored. Just checked the patch, perfect :-) You have a +1 from me, for what it's worth. Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Exposing object handles to userland
On Fri, Jul 31, 2015 at 4:53 PM, Nicolas Grekas nicolas.grekas+...@gmail.com wrote: I also know people that print_r($an_object) and parse the output just to extract the object handle from there... Crazy isn't it ? I plead guilty for doing this, but php let me no better choice for now ;) The attached patch removes the XOR hashing for the object handle (it's useless, the secret is trivially guessed after parsing the output of var_dump). It would be awesome if this patch could be applied for php 7.0! Cheers, Nicolas http://marc.info/?l=php-internalsm=141811755908008w=2 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I'd prefer to add a separate function spl_object_id, which directly returns the handle. This should supersede spl_object_hash in the long run. spl_object_hash does a bunch of pointless things that serve no purpose other than making the function slower and making the result more bulky. Nikita
[PHP-DEV] Re: Serializing exceptions
On 07/27/2015 02:08 AM, Stas Malyshev wrote: Hi! Looking into some issue, I've discovered that, to my surprise, Exceptions are serializable. Except that it doesn't always work of course (e.g. see http://stackoverflow.com/q/9747813/214196) because exceptions contain backtraces, and those can contain non-serializable objects. So in reality, you never know if you can serialize it or not. So, I wonder - would it be ok to make exceptions not serializable at all? I think that would prevent major WTF factor when people try it and it randomly fails. Since discussion on this did not lead to a definite conclusion, but I did not hear from anybody that they need serialized exceptions, and we keep getting bug reports about exception serialization and various issues triggered by it, I propose this change: https://github.com/php/php-src/pull/1442 Since it is kind of BC break (even though I assume it breaks something that needed not to be allowed in the first place) I'd like to merge it in 7.0.0. Please object if you think this should not be done and explain why. Otherwise I intend to merge it after a suitable wait and after adding necessary tests/docs. Thanks, I'm serializing exceptions in a current project, and I would much prefer losing `args` (the only part not always serializable) from the trace than not being able to serialize the exception at all. Making exceptions not serializable will just result in another userland wrapper, like a SuperException, that lets you serialize and unserialize with eval()'s. I think allowing the serialize to drop args in the trace and just include a warning in the docs about the serialization being lossy. My 2 cents. -- Stephen Coakley -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Throwable::addSuppressed()
On 07/28/2015 04:51 PM, Markus Malkusch wrote: Hi PHP So I read that there's this Throwable interface coming. Great! How about extending it with one further method: void Throwable::addSuppressed(Throwable exception) Semantic is the same as Java's Throwable.addSuppressed()¹. Why? Well consider a code fragment which wants to close a resource during an exception: } catch (Exception $e1) { try { $resource-close(); throw $e1; } catch (ResourceException $e2) { // The information about $e2 is lost. throw $e1; } } Currently PHP has no method to propagate both $e1 and $e2. With Throwable::addSuppressed() $e2 could be added as a suppressed exception to $e1: } catch (Exception $e1) { try { $resource-close(); } catch (ResourceException $e2) { e1-addSuppressed($e2); } throw $e1; } To make this information useful (for e.g. a logger) there's one further method needed: Throwable[] Throwable::getSuppressed() So PHP, what do you think, might a RFC find acceptance? Best wishes Markus Malkusch [1]: http://docs.oracle.com/javase/8/docs/api/java/lang/Throwable.html#addSuppressed-java.lang.Throwable- Interesting thought, but how is this different than including a previous throwable and throwing a new, more descriptive exception? } catch (Exception $e1) { try { $resource-close(); } catch (ResourceException $e2) { // Pass in $e2 as the previous exception in the chain. throw new ResourceFailedException($e1-getMessage(), $e1-getCode(), $e2); } } What you're describing doesn't seem like a common use case. Also, this is pretty nitpicky, but the above code might fit better with a finally block when closing resources: try { // Do something with $resource here... } finally { $resource-close(); } -- Stephen Coakley -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
Hi! Personally I feel the restoring them impossible argument weak, consider that we allow stuff like serializing resources without even a notice. Not sure what you mean by that. If you try to serialize resource, you just get an integer. Not ideal, as a remanant of the times in PHP where the approach was if it doesn't make sense, do whatever and hope the user is ok with that, but certainly it's not serializing resources. It's ignoring resources when serializing and producing integers instead. Replacing Exceptions with integers probably won't work that well :) Based on my own experiences where I had to fix multiple apps when we introduced the unserializable Closure (mostly error logger and debugging tools) which got passed as argument in the backtrace I would prefer if we could remove that restriction. I don't see how we can really remove the underlying problem - Exceptions contain backtraces, which means serializing them tries to serialize a ton of stuff that may be not only not serializable but outright dangerous to carry around - such as keys, passwords, etc. Given how many problems we have had with serialization of complex objects lately, and given that I still see absolutely no practical use of actually serializing exceptions I would rather remove it and reduce the vulnerable surface than keep dealing with dozens of issues that continue to pop up from that. BTW what you mean by unserializable Closure? As far as I know you can not serialize Closure. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Re: Serializing exceptions
Hi Stas, -Original Message- From: Stanislav Malyshev [mailto:smalys...@gmail.com] Sent: Tuesday, July 28, 2015 10:51 PM To: Marco Pivetta ocram...@gmail.com; Rowan Collins rowan.coll...@gmail.com Cc: Nikita Popov nikita@gmail.com; PHP Internals List internals@lists.php.net Subject: Re: [PHP-DEV] Re: Serializing exceptions Hi! New BC breaks in Beta? Why not? What's the problem with it? Beta is to identify issues with current code, serialized exceptions is an issue (as in, they don't work and lead to security problems and generally pointless). Dragging this known problem around for years makes no sense. I was looking through and seems that situation is not that bad. Reading http://fabien.potencier.org/php-serialization-stack-traces-and-exceptions.html it might be not that meaningful to break symphony. Having upset users is logic as long as we don't offer a solution, while the functionality itself seems to make sense. And after all serialization looks useful in some cases (logging at least). Maybe we should go more fine tuning way, maybe ignoring the thing that cannot be serialized. Say, serializing only a white list in this case? Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller m...@kelunik.com wrote: Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Now I see what do you mean by gracefully. TL;DR; It's app developer jobs to handle these fatal errors. Most fatal errors shouldn't be recovered by library. e.g. Fallback to non CSPRNG when CSPRNG is not available. IMO, library authors do not have to worry about errors like session's E_RECOVERABLE_ERROR or CSPRNG not available. These are fatal errors by its nature and app developer or system administrator job to handle them. We cannot blindly assume such fatal errors never happen in production apps, so we are better to give users a chance to return Nice error page/message when it happened. Anyway, we are better to move errors to exceptions at once and force all modules including 3rd party module to raise exceptions. i.e. Make php_error_docref() raise exceptions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
2015-07-31 23:36 GMT+02:00 Yasuo Ohgaki yohg...@ohgaki.net: Hi Niklas, On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller m...@kelunik.com wrote: Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Now I see what do you mean by gracefully. TL;DR; It's app developer jobs to handle these fatal errors. Nope. Most fatal errors shouldn't be recovered by library. e.g. Fallback to non CSPRNG when CSPRNG is not available. They should totally be handled. You need to catch the error and throw a defined exception, otherwise your public API will break if you choose to use another internal implementation. Additionally, you seem to assume that the library doesn't have to do things like cleanups in such a case. Regards, Niklas IMO, library authors do not have to worry about errors like session's E_RECOVERABLE_ERROR or CSPRNG not available. These are fatal errors by its nature and app developer or system administrator job to handle them. We cannot blindly assume such fatal errors never happen in production apps, so we are better to give users a chance to return Nice error page/message when it happened. Anyway, we are better to move errors to exceptions at once and force all modules including 3rd party module to raise exceptions. i.e. Make php_error_docref() raise exceptions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Nikita, On Thu, Jul 30, 2015 at 6:06 PM, Nikita Popov nikita@gmail.com wrote: On Thu, Jul 30, 2015 at 1:25 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi all, On Thu, Jul 30, 2015 at 7:44 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Thu, Jul 30, 2015 at 1:13 AM, Nikita Popov nikita@gmail.com wrote: Instead of continuing to use serialize_precision, which will produce unnecessarily long outputs for many values, why don't we just switch to using the 0 mode of zend_dtoa, i.e. to return the shortest output that is still accurate if interpreted in round-to-nearest. I think this is what everybody else is using when they convert floating point numbers to strings. I guess we may not be able to change normal floating point printing to use this, but this seems like the best mode for anything using serialize_precision now and everything that should be using it (like JSON, and queries, etc). I prefer your proposal! Your proposal is a lot better than now. Anyone has opinion for this? I'm writing the RFC and I would like to make this the first option. i.e. serialize_precision=0 uses zend_dtoa 0 mode for all data exchange functions (json/serialize/var_exrport. Anyone care about WDDX/XML_RPC?) I wrote draft RFC. https://wiki.php.net/rfc/precise_float_value Please comment. I would like to start RFC discussion shortly. Thank you. Nice idea about using a special serialize_precision value for this. This allows to keep BC for those that have tests for particular serialize output or similar things. I would suggest to default serialize_precision to -1 in PHP 7 -- if people want the previous behavior they can still have it, but I think -1 is the more reasonable default as it matches what one would naturally expect. I don't see the need for having a separate setting for JSON. Having a dozen different float precision settings will not help anyone. Thank you for your feedback. I agree it's better if PHP7 uses 0 mode by default. I'll update the RFC. I have things to do on this weekend. I'll try to write patch this weekend hopefully. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Scott, On Fri, Jul 31, 2015 at 6:33 AM, Scott Arciszewski sc...@paragonie.com wrote: On Jul 30, 2015 2:27 PM, Niklas Keller m...@kelunik.com wrote: I prefer Exception, too, because it's I/O related. @Scott: You can open votes on everything, doesn't matter, just create a page with a vote. I just don't know where to put it in the wiki, because it's not a RFC. Regards, Niklas I'm not sure how to do that. I have a noncritical security patch I intend to write for random_bytes() that I would like to submit as a PR but first I'd like to see what the resolution will be re: Exceptions. Also, merge conflicts aren't fun. ;) If there's anything I can do to get those two merged faster, please let me know. You can have RFC vote options like - Raise Error/Exception when CSPRNG is not available (Choice: Yes/No) - Use error(E_RECOVERABLE_ERROR) or exception(Whatever exception prefered) (Choice: Error/Exception) BTW, I prefer exception, but consistency is important also. My vote will be Yes and Error. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: Serializing exceptions
Hi! week or two and I serialize exceptions (excluding stack trace arguments) to send them back to the calling process to aid in debugging process failures. But then you don't need to serialize Exception. You need to send the text representation of Exception, for humans to look at, not the live Exception object. Sending the actual object would be next to impossible anyway (i.e. how you send over the live DB connection on DB query exception?). -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
Hi! this is where my point about we allow serializing resources just fine was aimed at. It's not just fine. It's it was there before we got smarter so now we can't change it without breaking too many things. But I think we did get smarter and no longer see silently putting whatever string in place of object while serializing as an acceptable method of serializing. nobody suggested replacing Exceptions with integers, but serializing Closure to Closure #1 or similar (albeit we could even do a better job, similarly how java8 supports lambda serialization). It's not serialization in any meaningful sense. It's replacing an object with a useless string value, silently, hoping the user didn't actually need it. If you didn't actually need it, why waste space serializing it at all? And why not to tell the user we can't actually serialize it? var_export() shares the same problem, should we then also try to remove __toString() from closures? toString is not serialization. toString is producing human-readable string representation - its very definition implies you lose all object identity and receive only string. It is completely different thing from serialization. Serialization is supposed to preserve the object so it can later be restored. Replacing object with toString() is in no way serialization. should give more rope for the developers to hang themselves but the current situations sucks (you can serialize basically anything And we have tons of problems with serializing basically anything, including a smorgasbord of security issues, because of that. Because of trying to unserialize objects that weren't supposed to be even serializable. Closure) and I think that your suggested way of solving the problem would create a medium sized BC break for little gain as it would only The gain is that we get rid of those issues and also get people to understand what serialization means (as I said it's not put an object, get a random string out and it's supposed to be two-way, not one-way). Yes, that means some objects could not be serialized. They *should not* be serialized, and the fail should be explicit, not in the form suddenly I get string instead of object, or, even worse, my app crashes randomly when I unserialize complex objects because somebody stored unexpected object in a property. that's true, but you would only remove a single attack vector via removing an otherwise useful feature I disagree with serializing exceptions being useful feature, and removing a single attack vector is one attack vector less. Its not like we can either remove all attack vectors at once (which as we both know is impossible) or never remove any of them. Removing them as we identify them is how we can do it. If we can remove a whole class of them instead of trying to fix a feature of no practical use that already proven to be source of trouble, it looks like a win to me. BTW what you mean by unserializable Closure? As far as I know you can not serialize Closure. I mean exactly that and btw. that was the original problem in the Well, if you mean serializing closure then you still can't do that. And I see no reason why you should be doing that. Stackoverflow post you cited in your opening post (http://stackoverflow.com/questions/9747813/storing-a-php-exception-object-in-a-database). I know. Serializing exceptions and storing them in the DB is *not* a good way to do logging. And us taking stance well, just serialize stuff and we'll throw in whatever strings there instead of objects encourages this wrong behavior. That's exactly what we need to fix. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
On Sat, Aug 1, 2015 at 1:10 AM, Stanislav Malyshev smalys...@gmail.com wrote: Hi! this is where my point about we allow serializing resources just fine was aimed at. It's not just fine. It's it was there before we got smarter so now we can't change it without breaking too many things. But I think we did get smarter and no longer see silently putting whatever string in place of object while serializing as an acceptable method of serializing. nobody suggested replacing Exceptions with integers, but serializing Closure to Closure #1 or similar (albeit we could even do a better job, similarly how java8 supports lambda serialization). It's not serialization in any meaningful sense. It's replacing an object with a useless string value, silently, hoping the user didn't actually need it. If you didn't actually need it, why waste space serializing it at all? And why not to tell the user we can't actually serialize it? var_export() shares the same problem, should we then also try to remove __toString() from closures? toString is not serialization. toString is producing human-readable string representation - its very definition implies you lose all object identity and receive only string. It is completely different thing from serialization. Serialization is supposed to preserve the object so it can later be restored. Replacing object with toString() is in no way serialization. should give more rope for the developers to hang themselves but the current situations sucks (you can serialize basically anything And we have tons of problems with serializing basically anything, including a smorgasbord of security issues, because of that. Because of trying to unserialize objects that weren't supposed to be even serializable. Closure) and I think that your suggested way of solving the problem would create a medium sized BC break for little gain as it would only The gain is that we get rid of those issues and also get people to understand what serialization means (as I said it's not put an object, get a random string out and it's supposed to be two-way, not one-way). Yes, that means some objects could not be serialized. They *should not* be serialized, and the fail should be explicit, not in the form suddenly I get string instead of object, or, even worse, my app crashes randomly when I unserialize complex objects because somebody stored unexpected object in a property. that's true, but you would only remove a single attack vector via removing an otherwise useful feature I disagree with serializing exceptions being useful feature, and removing a single attack vector is one attack vector less. Its not like we can either remove all attack vectors at once (which as we both know is impossible) or never remove any of them. Removing them as we identify them is how we can do it. If we can remove a whole class of them instead of trying to fix a feature of no practical use that already proven to be source of trouble, it looks like a win to me. BTW what you mean by unserializable Closure? As far as I know you can not serialize Closure. I mean exactly that and btw. that was the original problem in the Well, if you mean serializing closure then you still can't do that. And I see no reason why you should be doing that. Stackoverflow post you cited in your opening post ( http://stackoverflow.com/questions/9747813/storing-a-php-exception-object-in-a-database ). I know. Serializing exceptions and storing them in the DB is *not* a good way to do logging. And us taking stance well, just serialize stuff and we'll throw in whatever strings there instead of objects encourages this wrong behavior. That's exactly what we need to fix. -- Stas Malyshev smalys...@gmail.com for the record I won't reply to you until tomorrow, either you don't read what I'm writing or I'm too tired to properly articulate my point. if any chance it was the first, then please re-read my first email in this thread, thanks! -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 7:20 AM, Niklas Keller m...@kelunik.com wrote: 2015-07-31 23:36 GMT+02:00 Yasuo Ohgaki yohg...@ohgaki.net: Hi Niklas, On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller m...@kelunik.com wrote: Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Now I see what do you mean by gracefully. TL;DR; It's app developer jobs to handle these fatal errors. Nope. Most fatal errors shouldn't be recovered by library. e.g. Fallback to non CSPRNG when CSPRNG is not available. They should totally be handled. You need to catch the error and throw a defined exception, otherwise your public API will break if you choose to use another internal implementation. Additionally, you seem to assume that the library doesn't have to do things like cleanups in such a case. My thought is based on Design by Contract (Contract programming). When parameter or environment does not satisfy contract, contract error should be resulted in program/process termination. Fixing inappropriate parameter or environment is not library/framework author's responsibility, but the developer's. i.e. Caller(function/programmer/system admin) has the responsibility that satisfies parameter/environment requirement. If requirement is not met, it's perfectly OK for library/framework to raise fatal errors/exceptions. e.g. You need PHP 5.6 or greater error. Handling these fatal errors in a library/framework make code complex unnecessarily. More complex code has more chances to have bugs including security related bugs. Library/framework may simply raise error/exception telling users It's impossible to work. PHP is general programing language, so we have to consider long life applications such as standalone apps. I fully agree that exception is far easier for handling errors properly and keep app running. However, making randon_*() a special function does not worth it. IMHO. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: Serializing exceptions
Hi! As I have pointed out several times, it is only the 'args' section of the backtrace that ever contains unserializable items. The solution previous could too. In fact, right now, since you can unserialize exceptions, previous can contain literally anything and so can any other members. Also, user code can modify any protected properties too. DEBUG_BACKTRACE_IGNORE_ARGS in a debug_backtrace() call. IIRC the object of called methods is already excluded (equivalent to masking out DEBUG_PROVIDE_OBJECT) so what's left is all strings. I'm not sure how you arrived at the conclusion that all arguments in backtrace are strings. Arguments can be of any type. When printed, they are converted to strings, but they are not strings when stored. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 8:27 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: They should totally be handled. You need to catch the error and throw a defined exception, otherwise your public API will break if you choose to use another internal implementation. Additionally, you seem to assume that the library doesn't have to do things like cleanups in such a case. My thought is based on Design by Contract (Contract programming). When parameter or environment does not satisfy contract, contract error should be resulted in program/process termination. Fixing inappropriate parameter or environment is not library/framework author's responsibility, but the developer's. i.e. Caller(function/programmer/system admin) has the responsibility that satisfies parameter/environment requirement. If requirement is not met, it's perfectly OK for library/framework to raise fatal errors/exceptions. e.g. You need PHP 5.6 or greater error. I'll be more specific for CSPRNG not available error. If a author would like to handle the error and fallback to non crypt safe RNG, he/she should detect environment and execute alternative code for the environment. Catching exception and fallback to non crypt safe RNG is not optimal way for handling unsatisfactory environment. IMHO. If we need function that checks environment, we are better to provide one rather than let users to use exception. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: Serializing exceptions
On Fri, Jul 31, 2015 at 9:56 PM, Stanislav Malyshev smalys...@gmail.com wrote: Hi! Personally I feel the restoring them impossible argument weak, consider that we allow stuff like serializing resources without even a notice. Not sure what you mean by that. If you try to serialize resource, you just get an integer. Not ideal, as a remanant of the times in PHP where the approach was if it doesn't make sense, do whatever and hope the user is ok with that, but certainly it's not serializing resources. your argument was that we shouldn't allow serialization of such items where after the unserliazion you won't get the same thing back. this is where my point about we allow serializing resources just fine was aimed at. It's ignoring resources when serializing and producing integers instead. Replacing Exceptions with integers probably won't work that well :) nobody suggested replacing Exceptions with integers, but serializing Closure to Closure #1 or similar (albeit we could even do a better job, similarly how java8 supports lambda serialization). Based on my own experiences where I had to fix multiple apps when we introduced the unserializable Closure (mostly error logger and debugging tools) which got passed as argument in the backtrace I would prefer if we could remove that restriction. I don't see how we can really remove the underlying problem - Exceptions contain backtraces, which means serializing them tries to serialize a ton of stuff that may be not only not serializable but outright dangerous to carry around - such as keys, passwords, etc. var_export() shares the same problem, should we then also try to remove __toString() from closures? or try for looking for passwords or credit card data in the variable? I think that your argument is really streched. I'm not saying that we should give more rope for the developers to hang themselves but the current situations sucks (you can serialize basically anything but Closure) and I think that your suggested way of solving the problem would create a medium sized BC break for little gain as it would only fix a specific scenario (people like in your SO link in the opening post would still try to debug_backtrace() from their logger/debugger which would trigger the same problem that the Closure arg can't be serialized). Given how many problems we have had with serialization of complex objects lately, and given that I still see absolutely no practical use of actually serializing exceptions I would rather remove it and reduce the vulnerable surface than keep dealing with dozens of issues that continue to pop up from that. that's true, but you would only remove a single attack vector via removing an otherwise useful feature those issues would be still present, still reported, we should still fix then, even if you can't trigger them via an exception logger but a session unserialized. BTW what you mean by unserializable Closure? As far as I know you can not serialize Closure. I mean exactly that and btw. that was the original problem in the Stackoverflow post you cited in your opening post ( http://stackoverflow.com/questions/9747813/storing-a-php-exception-object-in-a-database ). -- Ferenc Kovács @Tyr43l - http://tyrael.hu
[PHP-DEV] Re: Throwable::addSuppressed()
Stephen Coakley: Interesting thought, but how is this different than including a previous throwable and throwing a new, more descriptive exception? Thank you for asking. If you would have again a look at the code fragment from the previous mail you will notice that there are two immutable exceptions: $e1 and $e2. But only one exception may leave the context. There is no possibility to attach $e2 to $e1, so some information will get lost. Also the scenario could be extended to n further exceptions, as n further resources might need some clean up operations. What you're describing doesn't seem like a common use case. I'd argue it is. Just imagine there are remaining resources which you really want to release in any case (e.g. releasing a lock). In the case of an exception (from the code before the releasing), this exception should leave the method plus the resources should be closed. Now what happens if closing those resources cause further exceptions? Only one exception may leave the method. Also other languages (e.g. Python and Java) decided that this use case is common enough. I'd like to illustrate it with a further more verbose example: public function withdraw($amount) { $this-lock(); try { // Let's assume anything here can throw an exception. $balance = $this-getBalance(); $balance -= $amount; $this-saveBalance($balance); $this-unlock(); // throws LockException } catch (LockException $e) { // Releasing failed, so nothing else to do and that's the only // exception, so we're good here. throw $e; } catch (\Exception $e1) { // unlock() was not called yet. assert ($this-isLocked()); try { $this-unlock(); // The resource was released and we can throw $e1. throw $e1; } catch (LockException $e2) { // Now I can only throw $e1 or $e2, that's why I add $e2. $e1-addSuppressed($e2); throw $e1; } } } the above code might fit better with a finally block when closing resources: try { // Do something with $resource here... } finally { $resource-close(); } Finally doesn't address this use case. With finally I would just exchange any origin exception with a possible exception from the close() operation. But I agree that try {} catch {try {catch {}} is quiet a huge boiler plate. That's why Java introduced together with Throwable::addSupressed() the try-with-resource statement. I would also love to see try-with-resource here, but for the beginning I'd like to focus on Throwable::addSupressed(). So do you still see no use case? Markus Malkusch -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Serializing exceptions
On 31 July 2015 20:56:30 BST, Stanislav Malyshev smalys...@gmail.com wrote: Hi! Personally I feel the restoring them impossible argument weak, consider that we allow stuff like serializing resources without even a notice. Not sure what you mean by that. If you try to serialize resource, you just get an integer. Not ideal, as a remanant of the times in PHP where the approach was if it doesn't make sense, do whatever and hope the user is ok with that, but certainly it's not serializing resources. It's ignoring resources when serializing and producing integers instead. Replacing Exceptions with integers probably won't work that well :) Based on my own experiences where I had to fix multiple apps when we introduced the unserializable Closure (mostly error logger and debugging tools) which got passed as argument in the backtrace I would prefer if we could remove that restriction. I don't see how we can really remove the underlying problem - Exceptions contain backtraces, which means serializing them tries to serialize a ton of stuff that may be not only not serializable but outright dangerous As I have pointed out several times, it is only the 'args' section of the backtrace that ever contains unserializable items. The solution is simply not to include that argument - equivalent to setting DEBUG_BACKTRACE_IGNORE_ARGS in a debug_backtrace() call. IIRC the object of called methods is already excluded (equivalent to masking out DEBUG_PROVIDE_OBJECT) so what's left is all strings. I would have thought that genuine use cases for extracting arbitrary arguments out of an exceptions backtrace would be pretty rare. Regards, -- Rowan Collins -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? -- Ferenc Kovács @Tyr43l - http://tyrael.hu
[PHP-DEV] Benchmark Results for PHP Master 2015-07-31
Results for project php-src-nightly, build date 2015-07-31 05:00:00+03:00 commit: ac87657d4219111ecc52f2b6f27f5a739f0379a3 revision_date: 2015-07-31 02:44:42+02: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.10% -0.68% Drupal 7.36 cgi -T1 php opc=on fps -0.49% -0.25% MediaWiki 1.23.9 cgi -T5000 php opc=on fps 0.39% -0.78% bench.php cgi -T1 php opc=on sec -0.39% -4.21% micro_bench.php cgi -T1 php opc=on sec -0.31% 2.64% mandelbrot.php cgi -T1 php opc=on sec -0.03% -1.79% 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] Core functions throwing exceptions in PHP7
Hi Niklas, On Fri, Jul 31, 2015 at 5:12 PM, Niklas Keller m...@kelunik.com wrote: I think the question is more whether Exception or Error (class, not E_RECOVERABLE_ERROR), to allow handling it gracefully. E_WARNING is too weak for CSPRNG not available. It's fatal error. I agree fatal errors should be able to handle gracefully if it is possible. That's the reason why I prepared a patch that changes session E_ERROR to E_RECOVERABLE_ERROR. https://github.com/php/php-src/compare/master...yohgaki:master-e_recoverable_error?expand=1 It's inspired by this thread :) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Regards, Niklas 2015-07-31 11:46 GMT+02:00 Yasuo Ohgaki yohg...@ohgaki.net: Hi Niklas, On Fri, Jul 31, 2015 at 5:12 PM, Niklas Keller m...@kelunik.com wrote: I think the question is more whether Exception or Error (class, not E_RECOVERABLE_ERROR), to allow handling it gracefully. E_WARNING is too weak for CSPRNG not available. It's fatal error. I agree fatal errors should be able to handle gracefully if it is possible. That's the reason why I prepared a patch that changes session E_ERROR to E_RECOVERABLE_ERROR. https://github.com/php/php-src/compare/master...yohgaki:master-e_recoverable_error?expand=1 It's inspired by this thread :) 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 30 Jul 2015, at 17:02, Joe Watkins pthre...@pthreads.org wrote: Even if some of those people replying haven't read or don't understand what you are suggesting, it is not a good tactic to assume that and reply with read the RFC. Hi Joe, Sorry about yesterday, I have done as you have said, and I have read those responses again, but unfortunately I still feel that I was right in my assumptions (notes below, maybe some interesting additions?). In general I have been getting very frustrated that no-one seems to really care about security (and to be fair, it has been annoying me for far too many years). Keeping in mind that I work with other developers who routinely keep introducing vulnerabilities like SQLi, XSS, CSRF... and doing annoying things like switching the CSP off, because they copy/paste some hideous/insecure JS, and can't be bothered to work out why the eval function isn't working. So maybe I should start a new thread, without Matt's subject (btw Matt, I really appreciate what you are trying todo, I disagree with the blocking element, and I think we can also address more than just SQL injection vulnerabilities)... maybe something like I've found this one weird trick that will fix every security issue... sorry, I hate that kind of approach, but if it gets a response, maybe its worth it :-) Craig -- http://news.php.net/php.internals/87346 From: Matt Tait Reply: N/A Original suggestion. -- http://news.php.net/php.internals/87348 From: Rowan Collins Reply: Matt Tait Suggestion to Matt to look for previous RFCs. -- http://news.php.net/php.internals/87350 From: Christoph Becker Reply: Matt Tait Points out the existing Taint RFC from 2008, by Wietse Venema. -- http://news.php.net/php.internals/87355 From: Pierre Joye Reply: Matt Tait Pointing out that there is more than SQLi (true), and it might be an impossible task (I disagree, as explained later). -- http://news.php.net/php.internals/87361 From: Thomas Bley Reply: Matt Tait Suggesting the use of bound parameters / prepared statements, which I agree is how it should be coded by the website developers, but I think the PHP language itself can help identify when this has not been done (just by raising a notice, not blocking anything). Also Thomas points out that static code analysis could identify these issues, but these are far from perfect, and PHP is in a much better position to be doing this... and has the advantage of being available to everyone. Just to note, I've played with a couple of static code analysers which cost in the region of £19,000 per year, and they still don't find the same number of escaping issues that my suggestion can find (they do look at other issues, so don't get rid of them, but this one thing that can be done better with the programming language itself). -- http://news.php.net/php.internals/87363 From: Lester Caine Reply: Matt Tait Short answer saying you should not use mysql... which I think is a bit short sighted (on the assumption that mysqli is a similar interface for most website developers). I'm not against using tools like ORMs (e.g. Doctrine), bound parameters / prepared statements, or even stored procedures... but they all have issues, and are all vulnerable to the kind of misuse I'm trying to address (explained in the next email). -- http://news.php.net/php.internals/87370 From: Me (Craig Francis) Reply: Lester Caine Just saying I disagree with Lester, and giving a very simple example of how developers can still mess up (once you start adding some abstractions, like an ORM, this becomes much harder to detect, and is why I'm so insistent that PHP needs to be checking for these issues). This is where I also suggest an alternative to Matt's original suggestion (something I posted 12 days before, and didn't really get any response). -- http://news.php.net/php.internals/87383 From: Lester Caine Reply: Me (Craig Francis) Kind of missing the point (maybe my example was too simple), and is talking about how Matts solution would cause problems. And I agree (sorry Matt), I don't think Matt's solution would work... but if Lester had read my reply, he would have seen that my suggestion was about education (but it can also help experienced developers as well). -- http://news.php.net/php.internals/87386 From: Me (Craig Francis) Reply: Lester Caine Trying to explain to Lester that I agree on education, and pointing out that my solution is different... and how (maybe my email was too long to read?).