Harden PHP C++ Code Against Non-Factory Constructions

php/src/Communicator.cpp

@@ -1496,6 +1496,9 @@ IcePHP::communicatorInit(void)
     INIT_CLASS_ENTRY(ce, "IcePHP_Communicator", _classMethods);
     ce.create_object = handleAlloc;
     communicatorClassEntry = zend_register_internal_class(&ce);
+    // Mark the class as final to prevent subclassing, and forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    communicatorClassEntry->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     // A null clone_obj makes the object uncloneable: clone throws an Error.
     _handlers.clone_obj = nullptr;

php/src/Connection.cpp

@@ -284,7 +284,10 @@ handleConnectionFreeStorage(zend_object* object)
 static int
 handleConnectionCompare(zval* zobj1, zval* zobj2)
 {
-    // PHP guarantees that the objects have the same class.
+    // PHP will call this fallback handler if either operand is not a connection.
+    // If both operands are connections, this is no-op and the rest of this function will be executed.
+    ZEND_COMPARE_OBJECTS_FALLBACK(zobj1, zobj2);
+
     Ice::ConnectionPtr con1 = Wrapper<Ice::ConnectionPtr>::value(zobj1);
     assert(con1);
     Ice::ConnectionPtr con2 = Wrapper<Ice::ConnectionPtr>::value(zobj2);
@@ -352,6 +355,9 @@ IcePHP::connectionInit(void)
     INIT_CLASS_ENTRY(ce, "IcePHP_Connection", _connectionClassMethods);
     ce.create_object = handleConnectionAlloc;
     connectionClassEntry = zend_register_internal_class(&ce);
+    // Mark the class as final to prevent subclassing, and forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    connectionClassEntry->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_connectionHandlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     // A null clone_obj makes the object uncloneable: clone throws an Error.
     _connectionHandlers.clone_obj = nullptr;
@@ -459,12 +465,7 @@ IcePHP::fetchConnection(zval* zv, Ice::ConnectionPtr& connection)
             invalidArgument("value is not a connection");
             return false;
         }
-        Wrapper<Ice::ConnectionPtr>* obj = Wrapper<Ice::ConnectionPtr>::extract(zv);
-        if (!obj)
-        {
-            return false;
-        }
-        connection = *obj->ptr;
+        connection = Wrapper<Ice::ConnectionPtr>::value(zv);
     }
     return true;
 }

php/src/Endpoint.cpp

@@ -221,6 +221,9 @@ IcePHP::endpointInit(void)
     INIT_CLASS_ENTRY(ce, "IcePHP_Endpoint", _endpointMethods);
     ce.create_object = handleEndpointAlloc;
     endpointClassEntry = zend_register_internal_class(&ce);
+    // Mark the class as final to prevent subclassing, and forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    endpointClassEntry->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_endpointHandlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     // A null clone_obj makes the object uncloneable: clone throws an Error.
     _endpointHandlers.clone_obj = nullptr;
@@ -232,6 +235,9 @@ IcePHP::endpointInit(void)
     INIT_NS_CLASS_ENTRY(ce, "Ice", "EndpointInfo", _endpointInfoMethods);
     ce.create_object = handleEndpointInfoAlloc;
     endpointInfoClassEntry = zend_register_internal_class(&ce);
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    endpointInfoClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_endpointInfoHandlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     // A null clone_obj makes the object uncloneable: clone throws an Error.
     _endpointInfoHandlers.clone_obj = nullptr;
@@ -244,6 +250,9 @@ IcePHP::endpointInit(void)
     INIT_NS_CLASS_ENTRY(ce, "Ice", "IPEndpointInfo", nullptr);
     ce.create_object = handleEndpointInfoAlloc;
     ipEndpointInfoClassEntry = zend_register_internal_class_ex(&ce, endpointInfoClassEntry);
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    ipEndpointInfoClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
     zend_declare_property_string(ipEndpointInfoClassEntry, "host", sizeof("host") - 1, "", ZEND_ACC_PUBLIC);
     zend_declare_property_long(ipEndpointInfoClassEntry, "port", sizeof("port") - 1, 0, ZEND_ACC_PUBLIC);
     zend_declare_property_string(
@@ -257,11 +266,17 @@ IcePHP::endpointInit(void)
     INIT_NS_CLASS_ENTRY(ce, "Ice", "TCPEndpointInfo", nullptr);
     ce.create_object = handleEndpointInfoAlloc;
     tcpEndpointInfoClassEntry = zend_register_internal_class_ex(&ce, ipEndpointInfoClassEntry);
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    tcpEndpointInfoClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
 
     // Define the UDPEndpointInfo class.
     INIT_NS_CLASS_ENTRY(ce, "Ice", "UDPEndpointInfo", nullptr);
     ce.create_object = handleEndpointInfoAlloc;
     udpEndpointInfoClassEntry = zend_register_internal_class_ex(&ce, ipEndpointInfoClassEntry);
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    udpEndpointInfoClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
     zend_declare_property_string(
         udpEndpointInfoClassEntry,
         "mcastInterface",
@@ -274,19 +289,28 @@ IcePHP::endpointInit(void)
     INIT_NS_CLASS_ENTRY(ce, "Ice", "WSEndpointInfo", nullptr);
     ce.create_object = handleEndpointInfoAlloc;
     wsEndpointInfoClassEntry = zend_register_internal_class_ex(&ce, endpointInfoClassEntry);
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    wsEndpointInfoClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
     zend_declare_property_string(wsEndpointInfoClassEntry, "resource", sizeof("resource") - 1, "", ZEND_ACC_PUBLIC);
 
     // Define the OpaqueEndpointInfo class.
     INIT_NS_CLASS_ENTRY(ce, "Ice", "OpaqueEndpointInfo", nullptr);
     ce.create_object = handleEndpointInfoAlloc;
     opaqueEndpointInfoClassEntry = zend_register_internal_class_ex(&ce, endpointInfoClassEntry);
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    opaqueEndpointInfoClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
     zend_declare_property_null(opaqueEndpointInfoClassEntry, "rawEncoding", sizeof("rawEncoding") - 1, ZEND_ACC_PUBLIC);
     zend_declare_property_null(opaqueEndpointInfoClassEntry, "rawBytes", sizeof("rawBytes") - 1, ZEND_ACC_PUBLIC);
 
     // Define the SSLEndpointInfo class.
     INIT_NS_CLASS_ENTRY(ce, "Ice", "SSLEndpointInfo", nullptr);
     ce.create_object = handleEndpointInfoAlloc;
     sslEndpointInfoClassEntry = zend_register_internal_class_ex(&ce, endpointInfoClassEntry);
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    sslEndpointInfoClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
 
     return true;
 }
@@ -322,12 +346,7 @@ IcePHP::fetchEndpoint(zval* zv, Ice::EndpointPtr& endpoint)
             invalidArgument("value is not an endpoint");
             return false;
         }
-        Wrapper<Ice::EndpointPtr>* obj = Wrapper<Ice::EndpointPtr>::extract(zv);
-        if (!obj)
-        {
-            return false;
-        }
-        endpoint = *obj->ptr;
+        endpoint = Wrapper<Ice::EndpointPtr>::value(zv);
     }
     return true;
 }

php/src/Logger.cpp

@@ -242,6 +242,9 @@ IcePHP::loggerInit(void)
     INIT_CLASS_ENTRY(ce, "IcePHP_Logger", _classMethods);
     ce.create_object = handleAlloc;
     loggerClassEntry = zend_register_internal_class(&ce);
+    // Mark the class as final to prevent subclassing, and forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    loggerClassEntry->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_loggerHandlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     // A null clone_obj makes the object uncloneable: clone throws an Error.
     _loggerHandlers.clone_obj = nullptr;

php/src/Properties.cpp

@@ -624,7 +624,7 @@ handleFreeStorage(zend_object* object)
 static zend_object*
 handleClone(zend_object* zobj)
 {
-    Ice::PropertiesPtr p = *Wrapper<Ice::PropertiesPtr>::fetch(zobj)->ptr;
+    Ice::PropertiesPtr p = Wrapper<Ice::PropertiesPtr>::value(zobj);
     assert(p);
     zval clone;
     if (!IcePHP::createProperties(&clone, p->clone()))
@@ -772,6 +772,9 @@ IcePHP::propertiesInit(void)
     INIT_CLASS_ENTRY(ce, "IcePHP_Properties", _classMethods);
     ce.create_object = handleAlloc;
     propertiesClassEntry = zend_register_internal_class(&ce);
+    // Mark the class as final to prevent subclassing, and forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    propertiesClassEntry->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     _handlers.clone_obj = handleClone;
     _handlers.free_obj = handleFreeStorage;

php/src/Proxy.cpp

@@ -1501,7 +1501,7 @@ static zend_object*
 handleClone(zend_object* zobj)
 {
     // Create a new object that shares a C++ proxy instance with this object.
-    ProxyPtr obj = *Wrapper<ProxyPtr>::fetch(zobj)->ptr;
+    ProxyPtr obj = Wrapper<ProxyPtr>::value(zobj);
     assert(obj);
     zval clone;
     if (!obj->clone(&clone, obj->proxy))
@@ -1520,9 +1520,8 @@ handleGetMethod(zend_object** object, zend_string* name, const zval* key)
     result = zend_get_std_object_handlers()->get_method(object, name, key);
     if (!result)
     {
-        Wrapper<ProxyPtr>* obj = Wrapper<ProxyPtr>::fetch(*object);
-        assert(obj->ptr);
-        ProxyPtr _this = *obj->ptr;
+        ProxyPtr _this = Wrapper<ProxyPtr>::value(*object);
+        assert(_this);
 
         ProxyInfoPtr info = _this->info;
         assert(info);
@@ -1543,16 +1542,12 @@ handleGetMethod(zend_object** object, zend_string* name, const zval* key)
 static int
 handleCompare(zval* zobj1, zval* zobj2)
 {
-    // PHP guarantees that the objects have the same class.
-    Wrapper<ProxyPtr>* obj1 = Wrapper<ProxyPtr>::extract(zobj1);
-    assert(obj1->ptr);
-    ProxyPtr _this1 = *obj1->ptr;
-    Ice::ObjectPrx prx1 = _this1->proxy;
+    // PHP will call this fallback handler if either operand is not a proxy.
+    // If both operands are proxies, this is no-op and the rest of this function will be executed.
+    ZEND_COMPARE_OBJECTS_FALLBACK(zobj1, zobj2);
 
-    Wrapper<ProxyPtr>* obj2 = Wrapper<ProxyPtr>::extract(zobj2);
-    assert(obj2->ptr);
-    ProxyPtr _this2 = *obj2->ptr;
-    Ice::ObjectPrx prx2 = _this2->proxy;
+    Ice::ObjectPrx prx1 = Wrapper<ProxyPtr>::value(zobj1)->proxy;
+    Ice::ObjectPrx prx2 = Wrapper<ProxyPtr>::value(zobj2)->proxy;
 
     if (prx1 == prx2)
     {
@@ -1677,7 +1672,9 @@ IcePHP::proxyInit(void)
     INIT_NS_CLASS_ENTRY(ce, "Ice", "ObjectPrx", _proxyMethods);
     ce.create_object = handleAlloc;
     proxyClassEntry = zend_register_internal_class(&ce);
-    // proxyClassEntry->ce_flags |= ZEND_ACC_EXPLICIT_ABSTRACT_CLASS;
+    // Forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    proxyClassEntry->ce_flags |= ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     _handlers.clone_obj = handleClone;
     _handlers.get_method = handleGetMethod;
@@ -1716,16 +1713,10 @@ IcePHP::fetchProxy(zval* zv, optional<Ice::ObjectPrx>& prx, ProxyInfoPtr& info,
             invalidArgument("value is not a proxy");
             return false;
         }
-        Wrapper<ProxyPtr>* obj = Wrapper<ProxyPtr>::extract(zv);
-        if (!obj)
-        {
-            runtimeError("unable to retrieve proxy object from object store");
-            return false;
-        }
-        assert(obj->ptr);
-        prx = (*obj->ptr)->proxy;
-        info = (*obj->ptr)->info;
-        comm = (*obj->ptr)->communicator;
+        ProxyPtr obj = Wrapper<ProxyPtr>::value(zv);
+        prx = obj->proxy;
+        info = obj->info;
+        comm = obj->communicator;
     }
     return true;
 }

php/src/Types.cpp

@@ -3647,11 +3647,17 @@ ZEND_FUNCTION(IcePHP_stringifyException)
     RETURN_STRINGL(str.c_str(), static_cast<int>(str.length()));
 }
 
+ZEND_METHOD(Ice_TypeInfo, __construct) { runtimeError("IcePHP_TypeInfo cannot be instantiated"); }
+
+ZEND_METHOD(Ice_ExceptionInfo, __construct) { runtimeError("IcePHP_ExceptionInfo cannot be instantiated"); }
+
 // Predefined methods for IcePHP_TypeInfo.
-static zend_function_entry _typeInfoMethods[] = {{0, 0, 0}};
+static zend_function_entry _typeInfoMethods[] = {
+    ZEND_ME(Ice_TypeInfo, __construct, ice_void_arginfo, ZEND_ACC_PRIVATE | ZEND_ACC_CTOR){0, 0, 0}};
 
 // Predefined methods for IcePHP_ExceptionInfo.
-static zend_function_entry _exceptionInfoMethods[] = {{0, 0, 0}};
+static zend_function_entry _exceptionInfoMethods[] = {
+    ZEND_ME(Ice_ExceptionInfo, __construct, ice_void_arginfo, ZEND_ACC_PRIVATE | ZEND_ACC_CTOR){0, 0, 0}};
 
 bool
 IcePHP::isUnset(zval* zv)
@@ -3678,6 +3684,9 @@ IcePHP::typesInit(INIT_FUNC_ARGS)
     INIT_CLASS_ENTRY(ce, "IcePHP_TypeInfo", _typeInfoMethods);
     ce.create_object = handleTypeInfoAlloc;
     typeInfoClassEntry = zend_register_internal_class(&ce);
+    // Mark the class as final to prevent subclassing, and forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    typeInfoClassEntry->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_typeInfoHandlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     // A null clone_obj makes the object uncloneable: clone throws an Error.
     _typeInfoHandlers.clone_obj = nullptr;
@@ -3688,6 +3697,9 @@ IcePHP::typesInit(INIT_FUNC_ARGS)
     INIT_CLASS_ENTRY(ce, "IcePHP_ExceptionInfo", _exceptionInfoMethods);
     ce.create_object = handleExceptionInfoAlloc;
     exceptionInfoClassEntry = zend_register_internal_class(&ce);
+    // Mark the class as final to prevent subclassing, and forbid serialization of the class.
+    // An instance created by anything other than our factory would have a null native pointer.
+    exceptionInfoClassEntry->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NOT_SERIALIZABLE;
     memcpy(&_exceptionInfoHandlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
     // A null clone_obj makes the object uncloneable: clone throws an Error.
     _exceptionInfoHandlers.clone_obj = nullptr;

php/src/Util.h

@@ -28,7 +28,7 @@ namespace IcePHP
             zend_object_std_init(&w->zobj, ce);
             object_properties_init(&w->zobj, ce);
 
-            w->ptr = 0;
+            w->ptr = nullptr;
             return w;
         }
 
@@ -43,7 +43,22 @@ namespace IcePHP
             return reinterpret_cast<Wrapper<T>*>(reinterpret_cast<char*>(object) - XtOffsetOf(Wrapper<T>, zobj));
         }
 
-        static T value(zval* zv) { return *extract(zv)->ptr; }
+        static T value(zval* zv) { return value(extract(zv)); }
+        static T value(zend_object* object) { return value(fetch(object)); }
+
+        static T value(Wrapper<T>* w)
+        {
+            if (!w->ptr)
+            {
+                // The underlying pointer is null, which means the PHP object was constructed outside the extension.
+                // We emit a non-returning error to the PHP interpreter, to avoid hitting the dereference below here.
+                zend_error_noreturn(
+                    E_ERROR,
+                    "%s(): the object was not created by the Ice extension",
+                    get_active_function_name());
+            }
+            return *w->ptr;
+        }
 
         // This must be last element in the struct
         zend_object zobj;

php/test/Ice/info/Client.php

@@ -27,6 +27,46 @@ function allTests($helper)
 {
     $communicator = $helper->communicator();
 
+    echo "testing that internal classes cannot be created outside the extension... ";
+    flush(); {
+        // These classes wrap native C++ state that only the extension's factory functions populate. An instance
+        // created any other way would carry a null native pointer and crash the first time it was used, so both
+        // direct construction (private constructor) and unserialization (ZEND_ACC_NOT_SERIALIZABLE) are rejected.
+        foreach (
+            [
+                "IcePHP_Communicator",
+                "IcePHP_Connection",
+                "IcePHP_Endpoint",
+                "IcePHP_Properties",
+                "IcePHP_Logger",
+                "IcePHP_TypeInfo",
+                "IcePHP_ExceptionInfo",
+                "Ice\\ObjectPrx",
+                "Ice\\EndpointInfo",
+                "Ice\\IPEndpointInfo",
+                "Ice\\TCPEndpointInfo",
+                "Ice\\UDPEndpointInfo",
+                "Ice\\WSEndpointInfo",
+                "Ice\\OpaqueEndpointInfo",
+                "Ice\\SSLEndpointInfo",
+            ] as $className
+        ) {
+            // Direct construction is blocked by the class's private constructor (throws Error).
+            try {
+                new $className();
+                test(false);
+            } catch (Error $ex) {
+            }
+            // Unserialization is blocked by ZEND_ACC_NOT_SERIALIZABLE (throws Exception).
+            try {
+                $o = unserialize('O:' . strlen($className) . ':"' . $className . '":0:{}');
+                test(false);
+            } catch (Exception $ex) {
+            }
+        }
+    }
+    echo "ok\n";
+
     echo "testing proxy endpoint information... ";
     flush(); {
         $p1 = $communicator->stringToProxy(

php/test/Ice/proxy/Client.php

@@ -426,6 +426,13 @@ function allTests($helper)
     test($derived == $base);
     test($cl == $derived);
 
+    // Comparing a proxy with an object of another class or with a value of another type must not crash: PHP's
+    // standard object comparison applies instead.
+    test($cl != new stdClass());
+    test(new stdClass() != $cl);
+    test($cl != $communicator);
+    test($cl == $cl->ice_toString());
+
     try {
         Test\MyInterfacePrxHelper::checkedCast($base, "facet");
         test(false);
@@ -467,6 +474,10 @@ function allTests($helper)
         test($cl->ice_fixed($connection)->ice_fixed($connection)->ice_getConnection() == $connection);
         $fixedConnection = $cl->ice_connectionId("ice_fixed")->ice_getConnection();
         test($cl->ice_fixed($connection)->ice_fixed($fixedConnection)->ice_getConnection() == $fixedConnection);
+        // Comparing a connection with an object of another class or with a value of another type must not crash.
+        test($connection != new stdClass());
+        test($connection != $cl);
+        test($connection == $connection->toString());
         try {
             $cl->ice_datagram()->ice_fixed($connection)->ice_ping();
         } catch (Exception $ex) {