diff --git a/php/src/Communicator.cpp b/php/src/Communicator.cpp index d78bfdf0531..83aadff115e 100644 --- a/php/src/Communicator.cpp +++ b/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; diff --git a/php/src/Connection.cpp b/php/src/Connection.cpp index fc1c904e3d5..fb6cbed4cd3 100644 --- a/php/src/Connection.cpp +++ b/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::value(zobj1); assert(con1); Ice::ConnectionPtr con2 = Wrapper::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* obj = Wrapper::extract(zv); - if (!obj) - { - return false; - } - connection = *obj->ptr; + connection = Wrapper::value(zv); } return true; } diff --git a/php/src/Endpoint.cpp b/php/src/Endpoint.cpp index dfc3c01e44a..79a9b6ee7df 100644 --- a/php/src/Endpoint.cpp +++ b/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,12 +289,18 @@ 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); @@ -287,6 +308,9 @@ IcePHP::endpointInit(void) 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* obj = Wrapper::extract(zv); - if (!obj) - { - return false; - } - endpoint = *obj->ptr; + endpoint = Wrapper::value(zv); } return true; } diff --git a/php/src/Logger.cpp b/php/src/Logger.cpp index 8838ebbea7e..7395616d013 100644 --- a/php/src/Logger.cpp +++ b/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; diff --git a/php/src/Properties.cpp b/php/src/Properties.cpp index c21930e3005..66fe538ec95 100644 --- a/php/src/Properties.cpp +++ b/php/src/Properties.cpp @@ -624,7 +624,7 @@ handleFreeStorage(zend_object* object) static zend_object* handleClone(zend_object* zobj) { - Ice::PropertiesPtr p = *Wrapper::fetch(zobj)->ptr; + Ice::PropertiesPtr p = Wrapper::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; diff --git a/php/src/Proxy.cpp b/php/src/Proxy.cpp index 66b1e78f1f8..9c1a26eb203 100644 --- a/php/src/Proxy.cpp +++ b/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::fetch(zobj)->ptr; + ProxyPtr obj = Wrapper::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* obj = Wrapper::fetch(*object); - assert(obj->ptr); - ProxyPtr _this = *obj->ptr; + ProxyPtr _this = Wrapper::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* obj1 = Wrapper::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* obj2 = Wrapper::extract(zobj2); - assert(obj2->ptr); - ProxyPtr _this2 = *obj2->ptr; - Ice::ObjectPrx prx2 = _this2->proxy; + Ice::ObjectPrx prx1 = Wrapper::value(zobj1)->proxy; + Ice::ObjectPrx prx2 = Wrapper::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& prx, ProxyInfoPtr& info, invalidArgument("value is not a proxy"); return false; } - Wrapper* obj = Wrapper::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::value(zv); + prx = obj->proxy; + info = obj->info; + comm = obj->communicator; } return true; } diff --git a/php/src/Types.cpp b/php/src/Types.cpp index 2a4c3528c4d..69d338f4976 100644 --- a/php/src/Types.cpp +++ b/php/src/Types.cpp @@ -3647,11 +3647,17 @@ ZEND_FUNCTION(IcePHP_stringifyException) RETURN_STRINGL(str.c_str(), static_cast(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; diff --git a/php/src/Util.h b/php/src/Util.h index 66577dffa83..9a941a792c3 100644 --- a/php/src/Util.h +++ b/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*>(reinterpret_cast(object) - XtOffsetOf(Wrapper, 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* 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; diff --git a/php/test/Ice/info/Client.php b/php/test/Ice/info/Client.php index c9dfca9970b..3ddce47dab0 100644 --- a/php/test/Ice/info/Client.php +++ b/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( diff --git a/php/test/Ice/proxy/Client.php b/php/test/Ice/proxy/Client.php index 0a1a30b6c68..526bb96f1e7 100644 --- a/php/test/Ice/proxy/Client.php +++ b/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) {