Protect against stack overflow if the user derives from Message. (#8248)
* Protect against stack overflow if the user derives from Message.
* For pure-PHP, change error into an exception.
diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c
index 0e61770..c8ea964 100644
--- a/php/ext/google/protobuf/message.c
+++ b/php/ext/google/protobuf/message.c
@@ -554,10 +554,32 @@
*/
PHP_METHOD(Message, __construct) {
Message* intern = (Message*)Z_OBJ_P(getThis());
- const Descriptor* desc = Descriptor_GetFromClassEntry(Z_OBJCE_P(getThis()));
+ const Descriptor* desc;
+ zend_class_entry *ce = Z_OBJCE_P(getThis());
upb_arena *arena = Arena_Get(&intern->arena);
zval *init_arr = NULL;
+ // This descriptor should always be available, as the generated __construct
+ // method calls initOnce() to load the descriptor prior to calling us.
+ //
+ // However, if the user created their own class derived from Message, this
+ // will trigger an infinite construction loop and blow the stack. We
+ // temporarily clear create_object to break this loop (see check in
+ // NameMap_GetMessage()).
+ PBPHP_ASSERT(ce->create_object == Message_create);
+ ce->create_object = NULL;
+ desc = Descriptor_GetFromClassEntry(ce);
+ ce->create_object = Message_create;
+
+ if (!desc) {
+ zend_throw_exception_ex(
+ NULL, 0,
+ "Couldn't find descriptor. Note only generated code may derive from "
+ "\\Google\\Protobuf\\Internal\\Message");
+ return;
+ }
+
+
Message_Initialize(intern, desc);
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|a!", &init_arr) == FAILURE) {
diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
index c02d2b4..64aadf9 100644
--- a/php/src/Google/Protobuf/Internal/Message.php
+++ b/php/src/Google/Protobuf/Internal/Message.php
@@ -93,8 +93,9 @@
$pool = DescriptorPool::getGeneratedPool();
$this->desc = $pool->getDescriptorByClassName(get_class($this));
if (is_null($this->desc)) {
- user_error(get_class($this) . " is not found in descriptor pool.");
- return;
+ throw new \InvalidArgumentException(
+ get_class($this) ." is not found in descriptor pool. " .
+ 'Only generated classes may derive from Message.');
}
foreach ($this->desc->getField() as $field) {
$setter = $field->getSetter();
diff --git a/php/tests/GeneratedClassTest.php b/php/tests/GeneratedClassTest.php
index c0de0ba..d1836a9 100644
--- a/php/tests/GeneratedClassTest.php
+++ b/php/tests/GeneratedClassTest.php
@@ -24,6 +24,13 @@
use PBEmpty\PBEcho\TestEmptyPackage;
use Php\Test\TestNamespace;
+# This is not allowed, but we at least shouldn't crash.
+class C extends \Google\Protobuf\Internal\Message {
+ public function __construct($data = null) {
+ parent::__construct($data);
+ }
+}
+
class GeneratedClassTest extends TestBase
{
@@ -1724,6 +1731,16 @@
}
#########################################################
+ # Test that we don't crash if users create their own messages.
+ #########################################################
+
+ public function testUserDefinedClass() {
+ # This is not allowed, but at least we shouldn't crash.
+ $this->expectException(Exception::class);
+ $p = new C();
+ }
+
+ #########################################################
# Test no segfault when error happens
#########################################################
