diff --git a/lib/HttpClient.php b/lib/HttpClient.php index 5d7a4823..deb1868d 100644 --- a/lib/HttpClient.php +++ b/lib/HttpClient.php @@ -236,11 +236,58 @@ private function resolveUrl(string $path, ?RequestOptions $options): string return $path; } + $this->assertSafePath($path); + $baseUrl = $options !== null && $options->baseUrl !== null ? $options->baseUrl : $this->baseUrl; $baseUrl = rtrim($baseUrl, '/'); return $baseUrl . '/' . ltrim($path, '/'); } + /** + * Reject paths whose segments could escape the intended endpoint once + * normalized by the HTTP transport or the receiving server. Service + * methods interpolate caller-supplied IDs into path templates without + * per-segment URL-encoding, so an unencoded "../" or embedded "?"/"#"/CRLF + * in a single ID would silently re-target the request at a different + * WorkOS resource under the application's authenticated API key. + * + * The check runs against the fully percent-decoded path so that encoded + * variants (`%2e%2e`, `%2f`, `%3f`, `%0d%0a`, ...) and double-encoded + * variants (`%252e%252e`, `%252f`, ...) cannot bypass it. + */ + private function assertSafePath(string $path): void + { + $decoded = self::decodeUntilStable($path); + + if (preg_match('/[\x00-\x1f?#]/', $decoded) === 1) { + throw new \InvalidArgumentException( + 'WorkOS request path contains a forbidden character (control character, "?", or "#"). Pass query parameters via the $query argument rather than embedding them in the path.', + ); + } + + foreach (explode('/', $decoded) as $segment) { + if ($segment === '.' || $segment === '..') { + throw new \InvalidArgumentException( + 'WorkOS request path contains a relative segment ("." or ".."). Refusing to send the request to avoid cross-resource redirection.', + ); + } + } + } + + /** + * Decode percent-encoded characters in a loop until the value stabilizes, + * closing double-encoding bypass vectors like `%252e%252e`. + */ + private static function decodeUntilStable(string $value): string + { + do { + $prev = $value; + $value = rawurldecode($value); + } while ($value !== $prev); + + return $value; + } + private function resolveTimeout(?RequestOptions $options): int { return $options !== null && $options->timeout !== null ? $options->timeout : $this->timeout; diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 34af98a6..429c5ec4 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -85,6 +85,88 @@ public function testBuildUrlAppendsQueryString(): void $this->assertSame('code', $query['response_type']); } + /** + * @return array + */ + public static function unsafePathProvider(): array + { + return [ + 'parent traversal segment' => ['connections/../webhook_endpoints/wh_target'], + 'leading parent traversal' => ['../webhook_endpoints/wh_target'], + 'current directory segment' => ['connections/./id'], + 'embedded query character' => ['connections/conn_123?override=1'], + 'embedded fragment character' => ['connections/conn_123#frag'], + 'embedded carriage return' => ["connections/conn_123\r\nHost: evil"], + 'embedded newline' => ["connections/conn_123\nfoo"], + 'embedded null byte' => ["connections/conn_123\x00"], + 'percent-encoded parent traversal lowercase' => ['connections/%2e%2e/webhook_endpoints/wh_target'], + 'percent-encoded parent traversal uppercase' => ['connections/%2E%2E/webhook_endpoints/wh_target'], + 'percent-encoded current directory segment' => ['connections/%2e/id'], + 'percent-encoded slash hides traversal' => ['connections%2F..%2Fwebhook_endpoints'], + 'percent-encoded slash hides encoded traversal' => ['connections%2F%2e%2e%2Fwebhook_endpoints'], + 'percent-encoded query character' => ['connections/conn_123%3Foverride=1'], + 'percent-encoded fragment character' => ['connections/conn_123%23frag'], + 'percent-encoded CRLF injection' => ['connections/conn_123%0D%0AHost:%20evil'], + 'percent-encoded null byte' => ['connections/conn_123%00'], + 'double-encoded parent traversal' => ['connections/%252e%252e/webhook_endpoints'], + 'double-encoded slash hides traversal' => ['connections%252F..%252Fwebhook_endpoints'], + 'double-encoded query character' => ['connections/conn_123%253Foverride=1'], + 'double-encoded null byte' => ['connections/conn_123%2500'], + ]; + } + + /** + * @dataProvider unsafePathProvider + */ + public function testRequestRejectsUnsafePaths(string $path): void + { + $client = new HttpClient( + apiKey: 'test_key', + clientId: null, + baseUrl: 'https://api.workos.com', + timeout: 10, + maxRetries: 0, + ); + + $this->expectException(\InvalidArgumentException::class); + $client->request('DELETE', $path); + } + + /** + * @dataProvider unsafePathProvider + */ + public function testBuildUrlRejectsUnsafePaths(string $path): void + { + $client = new HttpClient( + apiKey: 'test_key', + clientId: null, + baseUrl: 'https://api.workos.com', + timeout: 10, + maxRetries: 0, + ); + + $this->expectException(\InvalidArgumentException::class); + $client->buildUrl($path); + } + + public function testRequestAllowsSafePathsWithDotsInsideSegments(): void + { + $mock = new MockHandler([ + new Response(200, ['Content-Type' => 'application/json'], '{}'), + ]); + + $client = new HttpClient( + apiKey: 'test_key', + clientId: null, + baseUrl: 'https://api.workos.com', + timeout: 10, + maxRetries: 0, + handler: HandlerStack::create($mock), + ); + + $this->assertSame([], $client->request('GET', 'users/user.with.dots')); + } + public function testErrorResponseIncludesCodeAndError(): void { $body = json_encode([