Harden admin backend: SRI, GET-only display, IP input validation

src/Controller/Admin/IpsController.php

@@ -5,6 +5,7 @@
 
 use Cake\Cache\Cache;
 use Cake\Core\Configure;
+use Cake\Http\Exception\BadRequestException;
 use Cake\I18n\DateTime;
 use Captcha\Cache\RateLimitKey;
 
@@ -54,7 +55,7 @@ public function index(): void {
 	 * @return void
 	 */
 	public function view(?string $ip = null): void {
-		$ip = (string)$ip;
+		$ip = $this->assertValidIp($ip);
 		$query = $this->Captchas->find()
 			->where(['ip' => $ip])
 			->orderBy(['created' => 'DESC']);
@@ -87,7 +88,7 @@ public function view(?string $ip = null): void {
 	 */
 	public function reset(?string $ip = null) {
 		$this->request->allowMethod('post');
-		$ip = (string)$ip;
+		$ip = $this->assertValidIp($ip);
 
 		$count = $this->Captchas->reset($ip);
 
@@ -103,7 +104,7 @@ public function reset(?string $ip = null) {
 	 */
 	public function clearRateLimit(?string $ip = null) {
 		$this->request->allowMethod('post');
-		$ip = (string)$ip;
+		$ip = $this->assertValidIp($ip);
 
 		$rl = (array)Configure::read('Captcha.verifyRateLimit');
 		$cache = (string)($rl['cache'] ?? 'default');
@@ -172,6 +173,27 @@ protected function topIps(DateTime $since, ?bool $solved): array {
 		return $out;
 	}
 
+	/**
+	 * Validate the IP path argument as a real IPv4/IPv6 address.
+	 *
+	 * Defense-in-depth against arbitrary path strings being reflected into
+	 * queries, cache keys and flash messages.
+	 *
+	 * @param string|null $ip
+	 *
+	 * @throws \Cake\Http\Exception\BadRequestException
+	 *
+	 * @return string Normalized IP string.
+	 */
+	protected function assertValidIp(?string $ip): string {
+		$ip = (string)$ip;
+		if ($ip === '' || filter_var($ip, FILTER_VALIDATE_IP) === false) {
+			throw new BadRequestException(__d('captcha', 'Invalid IP address.'));
+		}
+
+		return $ip;
+	}
+
 	/**
 	 * @return array<int, array{ip: string, n: int}>
 	 */

src/Controller/CaptchaController.php

@@ -54,6 +54,8 @@ public function beforeFilter(EventInterface $event): void {
 	 * @return \Cake\Http\Response|null|void
 	 */
 	public function display($id = null) {
+		$this->request->allowMethod('get');
+
 		if ($id === null) {
 			$captcha = new Captcha();
 		} else {

templates/layout/captcha-admin.php

@@ -11,8 +11,16 @@
 	<meta charset="utf-8"/>
 	<meta name="viewport" content="width=device-width, initial-scale=1"/>
 	<title><?= $this->fetch('title') ?: __d('captcha', 'Captcha Admin') ?></title>
-	<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/css/bootstrap.min.css">
-	<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@fortawesome/fontawesome-free@6.5.2/css/all.min.css">
+	<link rel="stylesheet"
+		href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/css/bootstrap.min.css"
+		integrity="sha384-QWTKZyjpPEjISv5WaRU9OFeRpok6YctnYmDr5pNlyT2bRjXh0JMhjY6hW+ALEwIH"
+		crossorigin="anonymous"
+		referrerpolicy="no-referrer">
+	<link rel="stylesheet"
+		href="https://cdn.jsdelivr.net/npm/@fortawesome/fontawesome-free@6.5.2/css/all.min.css"
+		integrity="sha384-PPIZEGYM1v8zp5Py7UjFb79S58UeqCL9pYVnVPURKEqvioPROaVAJKKLzvH2rDnI"
+		crossorigin="anonymous"
+		referrerpolicy="no-referrer">
 	<style>
 		body { background: #f5f7fa; }
 		.captcha-admin-nav { background: #1f2937; }
@@ -58,6 +66,9 @@
 	<?= $this->fetch('content') ?>
 </div>
 
-<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/js/bootstrap.bundle.min.js"></script>
+<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/js/bootstrap.bundle.min.js"
+	integrity="sha384-YvpcrYf0tY3lHB60NNkmXc5s9fDVZLESaAA55NDzOxhy9GkcIdslK1eN7N6jIeHz"
+	crossorigin="anonymous"
+	referrerpolicy="no-referrer"></script>
 </body>
 </html>

tests/TestCase/Controller/Admin/IpsControllerTest.php

@@ -5,6 +5,7 @@
 
 use Cake\Cache\Cache;
 use Cake\Core\Configure;
+use Cake\Http\Exception\BadRequestException;
 use Cake\I18n\DateTime;
 use Cake\TestSuite\IntegrationTestTrait;
 use Cake\TestSuite\TestCase;
@@ -135,6 +136,36 @@ public function testViewUnknownIpShowsEmptyState() {
 		$this->assertResponseContains(__d('captcha', 'No captchas seen for this IP.'));
 	}
 
+	/**
+	 * @return void
+	 */
+	public function testViewRejectsNonIpString() {
+		$this->disableErrorHandlerMiddleware();
+		$this->expectException(BadRequestException::class);
+
+		$this->get(['plugin' => 'Captcha', 'prefix' => 'Admin', 'controller' => 'Ips', 'action' => 'view', 'not-an-ip']);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testResetRejectsNonIpString() {
+		$this->disableErrorHandlerMiddleware();
+		$this->expectException(BadRequestException::class);
+
+		$this->post(['plugin' => 'Captcha', 'prefix' => 'Admin', 'controller' => 'Ips', 'action' => 'reset', 'bogus-value']);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testClearRateLimitRejectsNonIpString() {
+		$this->disableErrorHandlerMiddleware();
+		$this->expectException(BadRequestException::class);
+
+		$this->post(['plugin' => 'Captcha', 'prefix' => 'Admin', 'controller' => 'Ips', 'action' => 'clearRateLimit', 'totally-not-ip']);
+	}
+
 	/**
 	 * @return void
 	 */

tests/TestCase/Controller/CaptchaControllerTest.php

@@ -3,6 +3,7 @@
 namespace Captcha\Test\TestCase\Controller;
 
 use Cake\Core\Configure;
+use Cake\Http\Exception\MethodNotAllowedException;
 use Cake\TestSuite\IntegrationTestTrait;
 use Cake\TestSuite\TestCase;
 
@@ -74,6 +75,17 @@ public function testDisplayExt() {
 		$this->assertResponseNotEmpty();
 	}
 
+	/**
+	 * @return void
+	 */
+	public function testDisplayRejectsPost() {
+		$this->disableErrorHandlerMiddleware();
+		$this->expectException(MethodNotAllowedException::class);
+
+		$id = '11111111-1111-4111-8111-111111111111';
+		$this->post(['plugin' => 'Captcha', 'controller' => 'Captcha', 'action' => 'display', $id]);
+	}
+
 	/**
 	 * @return void
 	 */