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

Client: php

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

Patch: Håkon Hitland <hakon.hitland@zedge.net>
Patch: Roy Sindre Norangshol <norangshol@zedge.net>

This closes #1314

1 files changed, 19 insertions, 0 deletions

diff --git a/test/php/TestClient.php b/test/php/TestClient.php
index 76fd9354d..1591027f4 100755
--- a/test/php/TestClient.php
+++ b/test/php/TestClient.php
@@ -492,6 +492,25 @@ try {
   print_r(' caught xception '.$x->errorCode.': '.$x->message."\n");
 }
 
+// Regression test for THRIFT-4263
+print_r("testBinarySerializer_Deserialize('foo')");
+try {
+  \Thrift\Serializer\TBinarySerializer::deserialize(base64_decode('foo'), \ThriftTest\Xtruct2::class);
+  echo "**FAILED**\n";
+  $exitcode |= ERR_STRUCTS;
+} catch (\Thrift\Exception\TTransportException $happy_exception) {
+  // We expected this due to binary data of base64_decode('foo') is less then 4
+  // bytes and it tries to find thrift version number in the transport by
+  // reading i32() at the beginning.  Casting to string validates that
+  // exception is still accessible in memory and not corrupted.  Without patch,
+  // PHP will error log that the exception doesn't have any tostring method,
+  // which is a lie due to corrupted memory.
+  for($i=99; $i > 0; $i--) {
+    (string)$happy_exception;
+  }
+  print_r("  SUCCESS\n");
+}
+
 /**
  * Normal tests done.
  */