Skip to content

Commit 47555b6

Browse files
authored
Merge commit from fork
fix: Enhance file validation in HttpRequest to prevent arbitrary file read vulnerabilities
2 parents 90e84ef + 9026da5 commit 47555b6

File tree

3 files changed

+62
-0
lines changed

3 files changed

+62
-0
lines changed

src/Utility/Assert.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3937,6 +3937,7 @@ public static function file($value, $message = ''): void
39373937

39383938
/**
39393939
* Will also pass if $value is a directory, use Assert::file() instead if you need to be sure it is a file.
3940+
* Prevents arbitrary file read vulnerabilities by rejecting paths with protocol separators.
39403941
*
39413942
* @param mixed $value
39423943
* @param string $message
@@ -3947,6 +3948,14 @@ public static function fileExists($value, $message = ''): void
39473948
{
39483949
self::string($value);
39493950

3951+
// Reject paths containing protocol separators to prevent arbitrary file read
3952+
if (str_contains((string) $value, '://')) {
3953+
self::reportInvalidArgument(sprintf(
3954+
$message ?: 'File paths with protocol separators ("://") are not allowed: %s',
3955+
self::valueToString($value),
3956+
));
3957+
}
3958+
39503959
if (! file_exists($value)) {
39513960
self::reportInvalidArgument(sprintf(
39523961
$message ?: 'The file %s does not exist.',

src/Utility/HttpRequest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Exception;
1111
use Http\Message\MultipartStream\MultipartStreamBuilder;
1212
use Psr\Http\Client\ClientExceptionInterface;
13+
1314
use Psr\Http\Message\{RequestInterface, ResponseInterface, StreamInterface};
1415

1516
use function defined;
@@ -130,6 +131,8 @@ public function addFile(
130131
?string $file_path,
131132
): self {
132133
if (null !== $file_path) {
134+
Assert::fileExists($file_path);
135+
Assert::readable($file_path);
133136
$this->files[$field] = $file_path;
134137
}
135138

tests/Unit/Utility/HttpRequestTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,56 @@ function(): HttpRequest {
176176
fn() => new HttpRequest($this->configuration, HttpClient::CONTEXT_GENERIC_CLIENT, 'get', '/' . uniqid())
177177
]]);
178178

179+
it('rejects file paths with protocol separators', function(): void {
180+
$client = new HttpRequest($this->configuration, HttpClient::CONTEXT_GENERIC_CLIENT, 'post', '/');
181+
182+
expect(fn() => $client->addFile('test', 'file:///etc/passwd'))
183+
->toThrow(\InvalidArgumentException::class, 'File paths with protocol separators ("://") are not allowed: "file:///etc/passwd"');
184+
185+
expect(fn() => $client->addFile('test', 'http://example.com/file.txt'))
186+
->toThrow(\InvalidArgumentException::class, 'File paths with protocol separators ("://") are not allowed: "http://example.com/file.txt"');
187+
188+
expect(fn() => $client->addFile('test', 'php://filter/convert.base64-encode/resource=/etc/passwd'))
189+
->toThrow(\InvalidArgumentException::class, 'File paths with protocol separators ("://") are not allowed: "php://filter/convert.base64-encode/resource=/etc/passwd"');
190+
});
191+
192+
it('rejects file paths for non-existent or unreadable files', function(): void {
193+
$client = new HttpRequest($this->configuration, HttpClient::CONTEXT_GENERIC_CLIENT, 'post', '/');
194+
195+
// Non-existent file
196+
expect(fn() => $client->addFile('test', '/non/existent/file.txt'))
197+
->toThrow(\InvalidArgumentException::class, 'The file "/non/existent/file.txt" does not exist.');
198+
199+
// Create a temp file with no read permissions to test unreadable file
200+
$tempFile = sys_get_temp_dir() . '/' . uniqid('auth0_test_');
201+
file_put_contents($tempFile, 'test content');
202+
chmod($tempFile, 0000); // Remove all permissions
203+
204+
try {
205+
expect(fn() => $client->addFile('test', $tempFile))
206+
->toThrow(\InvalidArgumentException::class);
207+
} finally {
208+
chmod($tempFile, 0666); // Restore permissions so we can delete it
209+
@unlink($tempFile);
210+
}
211+
});
212+
213+
it('allows valid local file paths', function(): void {
214+
$client = new HttpRequest($this->configuration, HttpClient::CONTEXT_GENERIC_CLIENT, 'post', '/');
215+
216+
// Create a temporary file that exists and is readable
217+
$tempFile = sys_get_temp_dir() . '/' . uniqid('auth0_test_');
218+
file_put_contents($tempFile, 'test content');
219+
220+
try {
221+
// This should not throw an exception
222+
$result = $client->addFile('test', $tempFile);
223+
expect($result)->toBeInstanceOf(HttpRequest::class);
224+
} finally {
225+
@unlink($tempFile);
226+
}
227+
});
228+
179229
it('throws a NetworkException when the underlying client raises a ClientExceptionInterface', function(HttpRequest $client): void {
180230
$client->call();
181231
})->with(['mocked client' => [

0 commit comments

Comments
 (0)