Harden PHP C++ Code Against Non-Factory Constructions

changelog.d/php/null-pointer-hardening.md

@@ -0,0 +1,3 @@
+- Fixed a crash where creating Ice extension objects (such as a proxy, communicator, or endpoint) outside of Ice could
+  crash the PHP process. Creating such objects via `new`, `unserialize()`, or reflection now raises an error instead.
+- Ice extension objects can no longer be serialized or deserialized.

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

@@ -352,6 +352,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;

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;
 }

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

@@ -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

@@ -1677,7 +1677,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;

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,20 @@ 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)
+        {
+            Wrapper<T>* w = extract(zv);
+            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(