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
diff --git a/test/php/TestClient.php b/test/php/TestClient.php
index 76fd935..1591027 100755
--- a/test/php/TestClient.php
+++ b/test/php/TestClient.php
@@ -492,6 +492,25 @@
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.
*/