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.
pull/8250/head
Joshua Haberman 4 years ago committed by GitHub
parent 10ecb080cb
commit d18df4f764
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 24
      php/ext/google/protobuf/message.c
  2. 5
      php/src/Google/Protobuf/Internal/Message.php
  3. 17
      php/tests/GeneratedClassTest.php

@ -554,10 +554,32 @@ static void Message_Initialize(Message *intern, const Descriptor *desc) {
*/
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) {

@ -93,8 +93,9 @@ class Message
$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();

@ -24,6 +24,13 @@ use Foo\testLowerCaseEnum;
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
{
@ -1723,6 +1730,16 @@ class GeneratedClassTest extends TestBase
$this->assertFalse($m->hasOneofString());
}
#########################################################
# 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
#########################################################

Loading…
Cancel
Save