Skip to content

Commit 1fa44f7

Browse files
bwoebibwoebi
committed
Fix GH-19044: Protected properties are not scoped according to their prototype
1 parent 2aeefb1 commit 1fa44f7

File tree

9 files changed

+212
-5
lines changed

9 files changed

+212
-5
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ PHP NEWS
1111
. Fixed bug GH-18907 (Leak when creating cycle in hook). (ilutov)
1212
. Fix OSS-Fuzz #427814456. (nielsdos)
1313
. Fix OSS-Fuzz #428983568 and #428760800. (nielsdos)
14+
. Fixed bug GH-19044 (Protected properties are not scoped according to their
15+
prototype). (Bob)
1416

1517
- Curl:
1618
. Fix memory leaks when returning refcounted value from curl callback.

‎Zend/tests/gh19044.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-19044: Protected properties must be scoped according to their prototype
3+
--FILE--
4+
<?php
5+
6+
abstract class P {
7+
protected $foo;
8+
}
9+
10+
class C1 extends P {
11+
protected $foo = 1;
12+
}
13+
14+
class C2 extends P {
15+
protected $foo = 2;
16+
17+
static function foo($c) { return $c->foo; }
18+
}
19+
20+
var_dump(C2::foo(new C2));
21+
var_dump(C2::foo(new C1));
22+
23+
?>
24+
--EXPECT--
25+
int(2)
26+
int(1)

‎Zend/tests/property_hooks/gh19044-1.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-19044: Protected properties must be scoped according to their prototype (common ancestor has a protected setter)
3+
--FILE--
4+
<?php
5+
6+
abstract class P {
7+
abstract public mixed $foo { get; }
8+
}
9+
10+
class C1 extends P {
11+
public protected(set) mixed $foo { get => 1; set {} }
12+
}
13+
14+
class GrandC1 extends C1 {
15+
public protected(set) mixed $foo { get => 2; set {} }
16+
}
17+
18+
class C2 extends C1 {
19+
static function foo($c) { return $c->foo += 1; }
20+
}
21+
22+
var_dump(C2::foo(new GrandC1));
23+
24+
?>
25+
--EXPECT--
26+
int(3)

‎Zend/tests/property_hooks/gh19044-2.phpt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
GH-19044: Protected properties must be scoped according to their prototype (common ancestor does not have a setter)
3+
--FILE--
4+
<?php
5+
6+
abstract class P {
7+
abstract public mixed $foo { get; }
8+
}
9+
10+
class C1 extends P {
11+
public mixed $foo { get => 1; }
12+
}
13+
14+
class GrandC1 extends C1 {
15+
public protected(set) mixed $foo { get => 2; set {} }
16+
}
17+
18+
class C2 extends C1 {
19+
static function foo($c) { return $c->foo += 1; }
20+
}
21+
22+
var_dump(C2::foo(new GrandC1));
23+
24+
?>
25+
--EXPECTF--
26+
Fatal error: Uncaught Error: Cannot modify protected(set) property GrandC1::$foo from scope C2 in %s:%d
27+
Stack trace:
28+
#0 %s(%d): C2::foo(Object(GrandC1))
29+
#1 {main}
30+
thrown in %s on line %d

‎Zend/tests/property_hooks/gh19044-3.phpt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
GH-19044: Protected properties must be scoped according to their prototype (abstract parent defining visibility only takes precedence)
3+
--FILE--
4+
<?php
5+
6+
abstract class P {
7+
abstract protected(set) mixed $foo { get; set; }
8+
}
9+
10+
class C1 extends P {
11+
public protected(set) mixed $foo { get => 2; set {} }
12+
}
13+
14+
class C2 extends P {
15+
public mixed $foo = 1;
16+
17+
static function foo($c) { return $c->foo += 1; }
18+
}
19+
20+
var_dump(C2::foo(new C1));
21+
22+
?>
23+
--EXPECT--
24+
int(3)

‎Zend/tests/property_hooks/gh19044-4.phpt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
GH-19044: Protected properties must be scoped according to their prototype (abstract parent sets protected(set) with not having grandparent a setter - both inherit from parent)
3+
--FILE--
4+
<?php
5+
6+
abstract class GP {
7+
abstract mixed $foo { get; }
8+
}
9+
10+
abstract class P extends GP {
11+
abstract protected(set) mixed $foo { get; set; }
12+
}
13+
14+
class C1 extends P {
15+
public protected(set) mixed $foo { get => 2; set {} }
16+
}
17+
18+
class C2 extends P {
19+
public mixed $foo = 1;
20+
21+
static function foo($c) { return $c->foo += 1; }
22+
}
23+
24+
var_dump(C2::foo(new C1));
25+
26+
?>
27+
--EXPECT--
28+
int(3)

‎Zend/tests/property_hooks/gh19044-5.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-19044: Protected properties must be scoped according to their prototype (abstract parent sets protected(set) with not having grandparent a setter - one inherits from grandparent)
3+
--FILE--
4+
<?php
5+
6+
abstract class GP {
7+
abstract mixed $foo { get; }
8+
}
9+
10+
abstract class P extends GP {
11+
abstract protected(set) mixed $foo { get; set; }
12+
}
13+
14+
class C1 extends P {
15+
public protected(set) mixed $foo { get => 2; set {} }
16+
}
17+
18+
class C2 extends GP {
19+
public mixed $foo = 1;
20+
21+
static function foo($c) { return $c->foo += 1; }
22+
}
23+
24+
var_dump(C2::foo(new C1));
25+
26+
?>
27+
--EXPECTF--
28+
Fatal error: Uncaught Error: Cannot modify protected(set) property C1::$foo from scope C2 in %s:%d
29+
Stack trace:
30+
#0 %s(%d): C2::foo(Object(C1))
31+
#1 {main}
32+
thrown in %s on line %d

‎Zend/tests/property_hooks/gh19044.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-19044: Protected properties must be scoped according to their prototype (hooks variation)
3+
--FILE--
4+
<?php
5+
6+
abstract class P {
7+
abstract protected $foo { get; }
8+
}
9+
10+
class C1 extends P {
11+
protected $foo = 1;
12+
}
13+
14+
class C2 extends P {
15+
protected $foo = 2;
16+
17+
static function foo($c) { return $c->foo; }
18+
}
19+
20+
var_dump(C2::foo(new C2));
21+
var_dump(C2::foo(new C1));
22+
23+
?>
24+
--EXPECT--
25+
int(2)
26+
int(1)

‎Zend/zend_object_handlers.c

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,20 @@ static zend_always_inline bool is_derived_class(const zend_class_entry *child_cl
282282
static zend_never_inline int is_protected_compatible_scope(const zend_class_entry *ce, const zend_class_entry *scope) /* {{{ */
283283
{
284284
return scope &&
285-
(is_derived_class(ce, scope) || is_derived_class(scope, ce));
285+
(ce == scope || is_derived_class(ce, scope) || is_derived_class(scope, ce));
286+
}
287+
/* }}} */
288+
289+
static zend_never_inline int is_asymmetric_set_protected_property_compatible_scope(const zend_property_info *info, const zend_class_entry *scope) /* {{{ */
290+
{
291+
zend_class_entry *ce;
292+
if (!(info->prototype->flags & ZEND_ACC_PROTECTED_SET) && info->hooks && info->hooks[ZEND_PROPERTY_HOOK_SET]) {
293+
zend_function *hookfn = info->hooks[ZEND_PROPERTY_HOOK_SET];
294+
ce = hookfn->common.prototype ? hookfn->common.prototype->common.scope : hookfn->common.scope;
295+
} else {
296+
ce = info->prototype->ce;
297+
}
298+
return is_protected_compatible_scope(ce, scope);
286299
}
287300
/* }}} */
288301

@@ -419,7 +432,7 @@ static zend_always_inline uintptr_t zend_get_property_offset(zend_class_entry *c
419432
}
420433
} else {
421434
ZEND_ASSERT(flags & ZEND_ACC_PROTECTED);
422-
if (UNEXPECTED(!is_protected_compatible_scope(property_info->ce, scope))) {
435+
if (UNEXPECTED(!is_protected_compatible_scope(property_info->prototype->ce, scope))) {
423436
goto wrong;
424437
}
425438
}
@@ -514,7 +527,7 @@ ZEND_API zend_property_info *zend_get_property_info(const zend_class_entry *ce,
514527
}
515528
} else {
516529
ZEND_ASSERT(flags & ZEND_ACC_PROTECTED);
517-
if (UNEXPECTED(!is_protected_compatible_scope(property_info->ce, scope))) {
530+
if (UNEXPECTED(!is_protected_compatible_scope(property_info->prototype->ce, scope))) {
518531
goto wrong;
519532
}
520533
}
@@ -585,7 +598,7 @@ ZEND_API bool ZEND_FASTCALL zend_asymmetric_property_has_set_access(const zend_p
585598
return true;
586599
}
587600
return EXPECTED((prop_info->flags & ZEND_ACC_PROTECTED_SET)
588-
&& is_protected_compatible_scope(prop_info->ce, scope));
601+
&& is_asymmetric_set_protected_property_compatible_scope(prop_info, scope));
589602
}
590603

591604
static void zend_property_guard_dtor(zval *el) /* {{{ */ {
@@ -2030,7 +2043,7 @@ ZEND_API zval *zend_std_get_static_property_with_info(zend_class_entry *ce, zend
20302043
zend_class_entry *scope = get_fake_or_executed_scope();
20312044
if (property_info->ce != scope) {
20322045
if (UNEXPECTED(property_info->flags & ZEND_ACC_PRIVATE)
2033-
|| UNEXPECTED(!is_protected_compatible_scope(property_info->ce, scope))) {
2046+
|| UNEXPECTED(!is_protected_compatible_scope(property_info->prototype->ce, scope))) {
20342047
if (type != BP_VAR_IS) {
20352048
zend_bad_property_access(property_info, ce, property_name);
20362049
}

0 commit comments

Comments
 (0)