Skip to content

Commit a9c3510

Browse files
henriquemoodyhenriquemoody
committed
Restrict CallableStringifier to closures to prevent data leakage
The `CallableStringifier` is undeniably useful, but its convenience comes at a security risk. By allowing strings and arrays to be interpreted as callables by default, we risk exposing sensitive data—like passwords or hostnames—hidden within those structures. Furthermore, it is often frustrating to have arbitrary strings automatically turned into callable representations. To fix this, I am moving to a "secure-by-default" stance. The stringifier now defaults to closure-only mode. Closures are significantly less likely to expose sensitive data in their contracts, making this a much safer baseline. I have made an exception for Fibers. Since a Fiber explicitly contains a callable, we know the usage is intentional, and being explicit about the underlying callable is important for debugging clarity. If someone still needs the original behavior, they can have it, but they must be intentional. They can restore the previous functionality by manually defining the `$closureOnly` parameter as `false` in the constructor.
1 parent 6d38b05 commit a9c3510

6 files changed

Lines changed: 160 additions & 98 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,23 +103,8 @@ echo stringify(new class { public int $property = 42; }) . PHP_EOL;
103103
echo stringify(new class extends WithProperties { }) . PHP_EOL;
104104
// `WithProperties@anonymous { +$publicProperty=true #$protectedProperty=42 }`
105105

106-
echo stringify('chr') . PHP_EOL;
107-
// `chr(int $codepoint): string`
108-
109-
echo stringify([new WithMethods(), 'publicMethod']) . PHP_EOL;
110-
// `WithMethods->publicMethod(Iterator&Countable $parameter): ?static`
111-
112-
echo stringify('WithMethods::publicStaticMethod') . PHP_EOL;
113-
// `WithMethods::publicStaticMethod(int|float $parameter): void`
114-
115-
echo stringify(['WithMethods', 'publicStaticMethod']) . PHP_EOL;
116-
// `WithMethods::publicStaticMethod(int|float $parameter): void`
117-
118-
echo stringify(new WithInvoke()) . PHP_EOL;
119-
// `WithInvoke->__invoke(int $parameter = 0): never`
120-
121106
echo stringify(static fn(int $foo): string => '') . PHP_EOL;
122-
// `function (int $foo): string`
107+
// `Closure { static fn (int $foo): string }`
123108

124109
echo stringify(new DateTime()) . PHP_EOL;
125110
// `DateTime { 2023-04-21T11:29:03+00:00 }`

‎phpcs.xml.dist‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,8 @@
3232
<rule ref="Generic.PHP.CharacterBeforePHPOpeningTag.Found">
3333
<exclude-pattern>tests/integration/</exclude-pattern>
3434
</rule>
35+
<rule ref="SlevomatCodingStandard.Functions.StaticClosure.ClosureNotStatic">
36+
<exclude-pattern>tests/integration</exclude-pattern>
37+
<exclude-pattern>tests/unit/Stringifiers/CallableStringifierTest.php</exclude-pattern>
38+
</rule>
3539
</ruleset>

‎src/Stringifiers/CallableStringifier.php‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -44,43 +44,47 @@ final class CallableStringifier implements Stringifier
4444
public function __construct(
4545
private readonly Stringifier $stringifier,
4646
private readonly Quoter $quoter,
47+
private readonly bool $closureOnly = true,
4748
) {
4849
}
4950

50-
public function stringify(mixed $raw, int $depth): string|null
51+
public function stringify(mixed $raw, int $depth): ?string
5152
{
52-
if (!is_callable($raw)) {
53-
return null;
54-
}
55-
5653
if ($raw instanceof Closure) {
5754
return $this->buildFunction(new ReflectionFunction($raw), $depth);
5855
}
5956

57+
if ($this->closureOnly || !is_callable($raw)) {
58+
return null;
59+
}
60+
6061
if (is_object($raw)) {
6162
return $this->buildMethod(new ReflectionMethod($raw, '__invoke'), $raw, $depth);
6263
}
6364

64-
if (is_array($raw) && is_object($raw[0]) && is_string($raw[1])) {
65-
return $this->buildMethod(new ReflectionMethod($raw[0], $raw[1]), $raw[0], $depth);
65+
if (is_array($raw) && is_object($raw[0])) {
66+
/** @var string $method */
67+
$method = $raw[1];
68+
69+
return $this->buildMethod(new ReflectionMethod($raw[0], $method), $raw[0], $depth);
6670
}
6771

68-
if (is_array($raw) && is_string($raw[0]) && is_string($raw[1])) {
69-
return $this->buildStaticMethod(new ReflectionMethod($raw[0], $raw[1]), $depth);
72+
if (is_array($raw) && is_string($raw[0])) {
73+
/** @var string $method */
74+
$method = $raw[1];
75+
76+
return $this->buildStaticMethod(new ReflectionMethod($raw[0], $method), $depth);
7077
}
7178

72-
if (is_string($raw) && str_contains($raw, ':')) {
79+
/** @var callable-string $raw */
80+
if (str_contains($raw, ':')) {
7381
/** @var class-string $class */
7482
$class = (string) strstr($raw, ':', true);
7583
$method = substr((string) strrchr($raw, ':'), 1);
7684

7785
return $this->buildStaticMethod(new ReflectionMethod($class, $method), $depth);
7886
}
7987

80-
if (!is_string($raw)) {
81-
return null;
82-
}
83-
8488
return $this->buildFunction(new ReflectionFunction($raw), $depth);
8589
}
8690

@@ -93,32 +97,31 @@ private function buildMethod(ReflectionMethod $reflection, object $object, int $
9397
{
9498
return $this->quoter->quote(
9599
sprintf('%s->%s', $this->getName($object), $this->buildSignature($reflection, $depth)),
96-
$depth,
100+
$depth
97101
);
98102
}
99103

100104
private function buildStaticMethod(ReflectionMethod $reflection, int $depth): string
101105
{
102106
return $this->quoter->quote(
103107
sprintf('%s::%s', $reflection->class, $this->buildSignature($reflection, $depth)),
104-
$depth,
108+
$depth
105109
);
106110
}
107111

108112
private function buildSignature(ReflectionFunctionAbstract $function, int $depth): string
109113
{
110-
$signature = $function->isClosure() ? 'function ' : $function->getName();
111-
$signature .= sprintf(
114+
$signature = sprintf(
112115
'(%s)',
113116
implode(
114117
', ',
115118
array_map(
116119
fn(ReflectionParameter $parameter): string => $this->buildParameter(
117120
$parameter,
118-
$depth + 1,
121+
$depth + 1
119122
),
120-
$function->getParameters(),
121-
),
123+
$function->getParameters()
124+
)
122125
),
123126
);
124127

@@ -128,7 +131,7 @@ private function buildSignature(ReflectionFunctionAbstract $function, int $depth
128131
' use ($%s)',
129132
implode(
130133
', $',
131-
array_keys($closureUsedVariables),
134+
array_keys($closureUsedVariables)
132135
),
133136
);
134137
}
@@ -138,7 +141,11 @@ private function buildSignature(ReflectionFunctionAbstract $function, int $depth
138141
$signature .= ': ' . $this->buildType($returnType, $depth);
139142
}
140143

141-
return $signature;
144+
if ($function->isClosure()) {
145+
return sprintf('Closure { %sfn%s }', $function->isStatic() ? 'static ' : '', $signature);
146+
}
147+
148+
return $function->getName() . $signature;
142149
}
143150

144151
private function buildParameter(ReflectionParameter $reflectionParameter, int $depth): string
@@ -163,7 +170,7 @@ private function buildParameter(ReflectionParameter $reflectionParameter, int $d
163170
return $parameter;
164171
}
165172

166-
private function buildValue(ReflectionParameter $reflectionParameter, int $depth): string|null
173+
private function buildValue(ReflectionParameter $reflectionParameter, int $depth): ?string
167174
{
168175
if (!$reflectionParameter->isDefaultValueAvailable()) {
169176
return $this->stringifier->stringify(null, $depth);
@@ -181,26 +188,19 @@ private function buildType(ReflectionType $raw, int $depth): string
181188
if ($raw instanceof ReflectionUnionType) {
182189
return implode(
183190
'|',
184-
array_map(fn(ReflectionType $type) => $this->buildType($type, $depth), $raw->getTypes()),
191+
array_map(fn(ReflectionType $type) => $this->buildType($type, $depth), $raw->getTypes())
185192
);
186193
}
187194

188195
if ($raw instanceof ReflectionIntersectionType) {
189196
return implode(
190197
'&',
191-
array_map(fn(ReflectionType $type) => $this->buildType($type, $depth), $raw->getTypes()),
198+
array_map(fn(ReflectionType $type) => $this->buildType($type, $depth), $raw->getTypes())
192199
);
193200
}
194201

195-
if ($raw instanceof ReflectionNamedType) {
196-
$type = $raw->getName();
197-
if ($raw->allowsNull()) {
198-
$type = sprintf('?%s', $type);
199-
}
200-
201-
return $type;
202-
}
202+
/** @var ReflectionNamedType $raw */
203203

204-
return '';
204+
return ($raw->allowsNull() ? '?' : '') . $raw->getName();
205205
}
206206
}

‎src/Stringifiers/CompositeStringifier.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ public static function createDefault(): self
6060
self::MAXIMUM_NUMBER_OF_PROPERTIES,
6161
),
6262
);
63-
$stringifier->prependStringifier($callableStringifier = new CallableStringifier($stringifier, $quoter));
64-
$stringifier->prependStringifier(new FiberObjectStringifier($callableStringifier, $quoter));
63+
$stringifier->prependStringifier(new CallableStringifier($stringifier, $quoter));
64+
$stringifier->prependStringifier(
65+
new FiberObjectStringifier(new CallableStringifier($stringifier, $quoter, closureOnly: false), $quoter)
66+
);
6567
$stringifier->prependStringifier(new EnumerationStringifier($quoter));
6668
$stringifier->prependStringifier(new ObjectWithDebugInfoStringifier($arrayStringifier, $quoter));
6769
$stringifier->prependStringifier(new ArrayObjectStringifier($arrayStringifier, $quoter));

‎tests/integration/stringify-callable.phpt‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,15 @@ declare(strict_types=1);
55

66
require 'vendor/autoload.php';
77

8-
$variable = new WithInvoke();
8+
$variable = true;
99

1010
outputMultiple(
11-
'chr',
12-
$variable,
13-
[new WithMethods(), 'publicMethod'],
14-
'WithMethods::publicStaticMethod',
15-
['WithMethods', 'publicStaticMethod'],
16-
static fn(int $foo): bool => (bool) $foo,
11+
fn(int $foo): bool => (bool) $foo,
1712
static function (int $foo) use ($variable): string {
1813
return $variable::class;
1914
},
2015
);
2116
?>
2217
--EXPECT--
23-
`chr(int $codepoint): string`
24-
`WithInvoke->__invoke(int $parameter = 0): never`
25-
`WithMethods->publicMethod(Iterator&Countable $parameter): ?static`
26-
`WithMethods::publicStaticMethod(int|float $parameter): void`
27-
`WithMethods::publicStaticMethod(int|float $parameter): void`
28-
`function (int $foo): bool`
29-
`function (int $foo) use ($variable): string`
18+
`Closure { fn(int $foo): bool }`
19+
`Closure { static fn(int $foo) use ($variable): string }`

0 commit comments

Comments
 (0)