Skip to content

Commit 9bc0a5a

Browse files
committed
magic methods: HTML-encode caller-supplied names in error messages
__get(), __call(), and __callStatic() interpolated the property or method name into their error text raw. Dynamic names can carry request data ($col = $_GET['sort']; $row->title->$col), and error handlers often echo messages into pages, so an unknown name like <script>...</script> reached the handler as a live tag. All three now encode the name first, same as orThrow() and orDie(); real names contain nothing to encode, so legitimate errors read the same as before.
1 parent 1b74602 commit 9bc0a5a

3 files changed

Lines changed: 45 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ These still work, they're just no longer featured in the docs - no changes requi
129129
`&amp;` in the Location header. A `SmartNull` argument counts as null
130130
(`new()` previously stored `""`, so `isNull()` reported a missing field as
131131
present)
132+
- The "Undefined property" and "Call to undefined method" error messages
133+
HTML-encode the caller-supplied name, matching `orThrow()` and `orDie()` -
134+
dynamic names can carry request data (`$row->title->$_GET['sort']`), and
135+
error handlers often echo messages into pages
132136
- `pregReplace()` returns null when the value causes a PCRE runtime failure
133137
(backtrack, recursion, or JIT stack limits on long values), so `->or()`
134138
fallbacks fire - previously only bad UTF-8 returned null and everything else

src/SmartString.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,6 +1136,10 @@ public function getIterator(): Iterator
11361136
*/
11371137
public function __get(string $property): SmartString
11381138
{
1139+
// SECURITY: encode the caller-supplied name - error handlers often echo messages into pages (see orThrow).
1140+
// Real method names encode to themselves, so the method_exists branch below is unaffected.
1141+
$property = htmlspecialchars($property, self::HTML_ENCODE_FLAGS, 'UTF-8');
1142+
11391143
// throw unknown property warning
11401144
// PHP Default Error: Warning: Undefined property: stdClass::$property in /path/to/template.php on line 28
11411145
if (method_exists($this, $property)) {
@@ -1231,6 +1235,7 @@ public function __call($method, $args): string|int|bool|null|float|SmartString
12311235

12321236
// throw unknown method Error
12331237
// PHP Default Error: Fatal error: Uncaught Error: Call to undefined method SmartString::method() in /path/to/template.php:17
1238+
$method = htmlspecialchars($method, self::HTML_ENCODE_FLAGS, 'UTF-8'); // SECURITY: encode the caller-supplied name - exception handlers often echo messages into pages (see orThrow)
12341239
$suggestion ??= "see the SmartString docs for available methods.";
12351240
$class = self::stripNamespace(self::class);
12361241
$error = "Call to undefined method $class->$method(), $suggestion\n" . self::occurredInFile();
@@ -1258,6 +1263,7 @@ public static function __callStatic($method, $args): mixed
12581263

12591264
// throw unknown method Error
12601265
// PHP Default Error: Fatal error: Uncaught Error: Call to undefined method SmartString::method() in /path/to/template.php:17
1266+
$method = htmlspecialchars($method, self::HTML_ENCODE_FLAGS, 'UTF-8'); // SECURITY: encode the caller-supplied name - exception handlers often echo messages into pages (see orThrow)
12611267
$baseClass = self::stripNamespace(self::class);
12621268
$error = "Call to undefined method $baseClass::$method(), see the SmartString docs for available methods.\n";
12631269
$error .= self::occurredInFile();

tests/Unit/MagicMethodsTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,19 @@ public function testGetWarnsOnUnknownProperty(): void
4646
$this->assertSmartString(null, $result);
4747
}
4848
49+
public function testGetEncodesAttackerSuppliedPropertyName(): void
50+
{
51+
// SECURITY: dynamic property names can carry request data
52+
// ($col = $_GET['sort']; $row->title->$col) and error handlers often
53+
// echo messages into pages, so the name must arrive encoded
54+
$property = '<script>alert(1)</script>';
55+
$result = $this->expectUserWarning(
56+
fn() => SmartString::new('x')->$property,
57+
"Undefined property: SmartString->&lt;script&gt;alert(1)&lt;/script&gt;\n"
58+
);
59+
$this->assertSmartString(null, $result);
60+
}
61+
4962
public function testGetInterpolatesAsEmptyStringAfterWarning(): void
5063
{
5164
// "$str->htmlEncode" in a string triggers __get, which returns
@@ -205,6 +218,17 @@ public function testUnknownMethodNameWithPercentReportsCleanly(): void
205218
);
206219
}
207220
221+
public function testUnknownMethodEncodesAttackerSuppliedName(): void
222+
{
223+
// SECURITY: same rule as __get() - exception handlers often echo
224+
// messages into pages, so the name must arrive encoded
225+
$method = '<script>alert(1)</script>';
226+
$this->assertUndefinedMethodError(
227+
"Call to undefined method SmartString->&lt;script&gt;alert(1)&lt;/script&gt;(), see the SmartString docs for available methods.\n",
228+
fn() => SmartString::new('x')->$method()
229+
);
230+
}
231+
208232
//endregion
209233
//region __callStatic()
210234
@@ -235,6 +259,17 @@ public function testUnknownStaticMethodPointsToDocs(): void
235259
);
236260
}
237261
262+
public function testUnknownStaticMethodEncodesAttackerSuppliedName(): void
263+
{
264+
// SECURITY: same rule as __get() - exception handlers often echo
265+
// messages into pages, so the name must arrive encoded
266+
$method = '<script>alert(1)</script>';
267+
$this->assertUndefinedMethodError(
268+
"Call to undefined method SmartString::&lt;script&gt;alert(1)&lt;/script&gt;(), see the SmartString docs for available methods.\n",
269+
fn() => SmartString::$method()
270+
);
271+
}
272+
238273
//endregion
239274
//region __debugInfo()
240275

0 commit comments

Comments
 (0)