[jira] [Commented] (THRIFT-4263) Fix use after free bug for thrown exceptions

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16124781#comment-16124781
 ] 

ASF GitHub Bot commented on THRIFT-4263:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1314


> Fix use after free bug for thrown exceptions
> 
>
> Key: THRIFT-4263
> URL: https://issues.apache.org/jira/browse/THRIFT-4263
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0, 0.11.0
> Environment: Verified both on Debian unstable and Centos 7 under 
> valgrind
>Reporter: Roy Sindre Norangshol
>Assignee: James E. King, III
>Priority: Critical
> Fix For: 0.11.0
>
>
> Fix use after free bug for thrown exceptions
> Exceptions thrown through PHPExceptionWrapper are prematurely freed at
> the end of the catch block, even though zend_throw_exception_object
> expects to take ownership of the value.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4263) Fix use after free bug for thrown exceptions

[James E. King, III (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123842#comment-16123842
 ] 

James E. King, III commented on THRIFT-4263:


Updating the distro used in the build process is a good idea.  We want coverage 
for some older environments however so we'll have to use a mixture of both.  
That deserves a separate story, however.  For this one, I was looking for 
documentation on zend_throw_exception_object and its requirements to verify 
your ownership assertions in the description however I could not find it?


> Fix use after free bug for thrown exceptions
> 
>
> Key: THRIFT-4263
> URL: https://issues.apache.org/jira/browse/THRIFT-4263
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0, 0.11.0
> Environment: Verified both on Debian unstable and Centos 7 under 
> valgrind
>Reporter: Roy Sindre Norangshol
>Priority: Critical
>
> Fix use after free bug for thrown exceptions
> Exceptions thrown through PHPExceptionWrapper are prematurely freed at
> the end of the catch block, even though zend_throw_exception_object
> expects to take ownership of the value.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4263) Fix use after free bug for thrown exceptions

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16102873#comment-16102873
 ] 

ASF GitHub Bot commented on THRIFT-4263:


Github user norrs commented on the issue:

https://github.com/apache/thrift/pull/1314
  
Test failures seems unrelated, how do we retrigger em to be run again? 


> Fix use after free bug for thrown exceptions
> 
>
> Key: THRIFT-4263
> URL: https://issues.apache.org/jira/browse/THRIFT-4263
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0, 0.11.0
> Environment: Verified both on Debian unstable and Centos 7 under 
> valgrind
>Reporter: Roy Sindre Norangshol
>Priority: Critical
>
> Fix use after free bug for thrown exceptions
> Exceptions thrown through PHPExceptionWrapper are prematurely freed at
> the end of the catch block, even though zend_throw_exception_object
> expects to take ownership of the value.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4263) Fix use after free bug for thrown exceptions

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16102123#comment-16102123
 ] 

ASF GitHub Bot commented on THRIFT-4263:


Github user norrs commented on the issue:

https://github.com/apache/thrift/pull/1314
  
Side story:

Ok, I was hoping travis would run the cross compile tests with php7, but 
sadly it doesn't. As I was hoping the build would fail with the test case 
before submitting, as this is what we've used for testing it under valgrind.


Currently cross compile builds runs with ubuntu 1404 trusty. For thrift as 
a project you might consider trying to maybe bump this dependency, as 
maintenance updates for 1404 is more or less going away this year according to 
https://www.ubuntu.com/info/release-end-of-life . 

You will also need min 1604 for php7 packages. Sadly the test suite really 
doesn't run with newer versions, at least python tests seems to start failing. 
Another problem is the system tests with (cross-compile) doesn't use cmake yet, 
which seems to have an idea about being able to select which libraries you want 
to include in the build. 

Also note that PHP5 != PHP7 , and you probably want to focus new 
development towards PHP7. http://php.net/supported-versions.php . 

Ideally CI should maybe build with latest distros where developers hang and 
live :-)

Anyhow, me and @zhaakhi rolled out this in production for our systems and 
haven't noticed any segfaults since we deployed it 24 hours ago. But we did 
revert https://github.com/apache/thrift/pull/1215 in our fork that we have 
deployed, since we noticed memory leaks from it. Unsure if it was due to me  
not ensuring newer thrift compiler under testing, but didn't want to risk it. 
I've also left a note about it in 
https://issues.apache.org/jira/browse/THRIFT-4126 . 


> Fix use after free bug for thrown exceptions
> 
>
> Key: THRIFT-4263
> URL: https://issues.apache.org/jira/browse/THRIFT-4263
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0, 0.11.0
> Environment: Verified both on Debian unstable and Centos 7 under 
> valgrind
>Reporter: Roy Sindre Norangshol
>Priority: Critical
>
> Fix use after free bug for thrown exceptions
> Exceptions thrown through PHPExceptionWrapper are prematurely freed at
> the end of the catch block, even though zend_throw_exception_object
> expects to take ownership of the value.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-4263) Fix use after free bug for thrown exceptions

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16101879#comment-16101879
 ] 

ASF GitHub Bot commented on THRIFT-4263:


GitHub user norrs opened a pull request:

https://github.com/apache/thrift/pull/1314

THRIFT-4263: Fix use after free bug for thrown exceptions

Exceptions thrown through PHPExceptionWrapper are prematurely freed at the 
end
of the catch block, even though zend_throw_exception_object expects to take
ownership of the value.

Ensure we free return_value in case of exceptions

Test binary deserialization of insufficient data which verifies we can cast
exception to string to verify against memory corruption when transport casts
exceptions.

Patch: Håkon Hitland 
Patch: Roy Sindre Norangshol 

This closes #4263

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/norrs/thrift THRIFT-4263

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1314.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1314


commit 7e64cc664170999e8224c7df4a689efebc55aea5
Author: Roy Sindre Norangshol 
Date:   2017-07-26T16:19:38Z

THRIFT-4263: Test case for 'Fix use after free bug for thrown exceptions'

Test binary deserialization of insufficient data which verifies we can cast
exception to string to verify against memory corruption when transport casts
exceptions.

Patch: Håkon Hitland 
Patch: Roy Sindre Norangshol 




> Fix use after free bug for thrown exceptions
> 
>
> Key: THRIFT-4263
> URL: https://issues.apache.org/jira/browse/THRIFT-4263
> Project: Thrift
>  Issue Type: Bug
>  Components: PHP - Library
>Affects Versions: 0.10.0, 0.11.0
> Environment: Verified both on Debian unstable and Centos 7 under 
> valgrind
>Reporter: Roy Sindre Norangshol
>Priority: Critical
>
> Fix use after free bug for thrown exceptions
> Exceptions thrown through PHPExceptionWrapper are prematurely freed at
> the end of the catch block, even though zend_throw_exception_object
> expects to take ownership of the value.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)