From ebf3cfc552bb897aafdcd077d5549f2f3540c55d Mon Sep 17 00:00:00 2001 From: Chris Portscheller Date: Sun, 4 Oct 2026 12:33:25 -0500 Subject: [PATCH] fix: BotDetector no longer trusts forwarded headers it was never told to trust When signals arrived without ip_address, BotDetector::analyze() fell back to a private resolver that took CF-Connecting-IP or the leftmost X-Forwarded-For from any client. That address is the one a claimed good bot is verified against. It now uses SignalCollector::getIP(), the trusted-proxy-aware resolver the rest of the plugin uses. A valid supplied ip_address is still kept. WebDecoy_Detector also built its BotDetector without the trusted proxies, so on a site behind Cloudflare it verified good bots against the edge address. It now passes the configured proxies. Closes #85. Refs WebDecoy/app#875. --- includes/class-webdecoy-detector.php | 3 + sdk/src/BotDetector.php | 40 +------ tests/BotDetectorIpTest.php | 161 +++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 35 deletions(-) create mode 100644 tests/BotDetectorIpTest.php diff --git a/includes/class-webdecoy-detector.php b/includes/class-webdecoy-detector.php index fc4f865..28fc40e 100644 --- a/includes/class-webdecoy-detector.php +++ b/includes/class-webdecoy-detector.php @@ -51,6 +51,9 @@ public function __construct(?array $options = null) 'allow_social_bots' => $this->options['allow_social_bots'] ?? true, 'block_ai_crawlers' => $this->options['block_ai_crawlers'] ?? false, 'custom_allowlist' => $this->options['custom_allowlist'] ?? [], + // Same resolver as the plugin's get_client_ip(): without this, a site + // behind Cloudflare verified good bots against the edge address (#85). + 'trusted_proxies' => function_exists('webdecoy_plugin_trusted_proxies') ? webdecoy_plugin_trusted_proxies() : [], // Per-path crawler refusals set in WebDecoy Cloud (#995); null // when not connected or the cached copy is too old to trust. 'cloud_policy' => class_exists('WebDecoy_Cloud_Policy') ? WebDecoy_Cloud_Policy::get_policy() : null, diff --git a/sdk/src/BotDetector.php b/sdk/src/BotDetector.php index 0d30b1c..66324d3 100644 --- a/sdk/src/BotDetector.php +++ b/sdk/src/BotDetector.php @@ -187,9 +187,12 @@ public function analyze(?array $signals = null, ?string $clientIP = null): Detec $signals = $this->signalCollector->collect(); } - // Get client IP if not provided + // Get client IP if not provided. A valid supplied ip_address is kept; + // anything else goes to the collector's trusted-proxy-aware resolver, + // never to the raw forwarding headers, which any client can set (#85). if ($clientIP === null) { - $clientIP = $signals['ip_address'] ?? $this->getClientIP(); + $supplied = is_string($signals['ip_address'] ?? null) ? trim($signals['ip_address']) : ''; + $clientIP = filter_var($supplied, FILTER_VALIDATE_IP) ? $supplied : $this->signalCollector->getIP(); } $result = new DetectionResult(0, []); @@ -303,39 +306,6 @@ public function analyze(?array $signals = null, ?string $clientIP = null): Detec return $result; } - /** - * Get client IP address - * - * @return string - */ - private function getClientIP(): string - { - // Priority: CF-Connecting-IP > X-Forwarded-For > X-Real-IP > REMOTE_ADDR - $headers = [ - 'HTTP_CF_CONNECTING_IP', - 'HTTP_X_FORWARDED_FOR', - 'HTTP_X_REAL_IP', - 'REMOTE_ADDR', - ]; - - foreach ($headers as $header) { - // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- IP validated with FILTER_VALIDATE_IP below - if (!empty($_SERVER[$header])) { - // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized, WordPress.Security.ValidatedSanitizedInput.MissingUnslash -- WP path unslashes + sanitizes; the standalone fallback trims the raw value because wp_unslash() is unavailable outside WordPress - $ip = function_exists('sanitize_text_field') ? sanitize_text_field(wp_unslash($_SERVER[$header])) : trim($_SERVER[$header]); - // X-Forwarded-For can contain multiple IPs - if (strpos($ip, ',') !== false) { - $ip = trim(explode(',', $ip)[0]); - } - if (filter_var($ip, FILTER_VALIDATE_IP)) { - return $ip; - } - } - } - - return '0.0.0.0'; - } - /** * Calculate threat score from signals * diff --git a/tests/BotDetectorIpTest.php b/tests/BotDetectorIpTest.php new file mode 100644 index 0000000..c4f89c2 --- /dev/null +++ b/tests/BotDetectorIpTest.php @@ -0,0 +1,161 @@ +verifiedIp = $ip; + return ['verified' => false, 'hostname' => null, 'reason' => 'spy']; + } +} + +const BOT_DETECTOR_IP_GOOGLEBOT = 'Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)'; + +/** + * The IP analyze() verified a Googlebot request against. + * + * @param array $server Request $_SERVER + * @param string[] $proxies Trusted proxies + * @param array|null $signals Signals passed to analyze(), or null to collect + */ +function bot_detector_verified_ip(array $server, array $proxies, ?array $signals = null): ?string +{ + $saved = $_SERVER; + $_SERVER = $server + ['HTTP_USER_AGENT' => BOT_DETECTOR_IP_GOOGLEBOT, 'REQUEST_URI' => '/']; + try { + $detector = new \WebDecoy\BotDetector(['trusted_proxies' => $proxies]); + $spy = new BotDetectorIpSpy(); + $prop = new ReflectionProperty(\WebDecoy\BotDetector::class, 'goodBotList'); + if (PHP_VERSION_ID < 80100) { + $prop->setAccessible(true); + } + $prop->setValue($detector, $spy); + $detector->analyze($signals); + return $spy->verifiedIp; + } finally { + $_SERVER = $saved; + } +} + +/** Signals without ip_address, as a caller that collected its own would pass. */ +function bot_detector_signals_without_ip(): array +{ + return ['user_agent' => BOT_DETECTOR_IP_GOOGLEBOT, 'request_path' => '/']; +} + +$spoofed = [ + 'REMOTE_ADDR' => '203.0.113.5', + 'HTTP_CF_CONNECTING_IP' => '192.0.2.66', + 'HTTP_X_FORWARDED_FOR' => '192.0.2.77, 198.51.100.1', + 'HTTP_X_REAL_IP' => '192.0.2.88', +]; + +TestRunner::test('with no trusted proxy, forwarding headers cannot choose the verified IP', function () use ($spoofed) { + TestRunner::assertSame('203.0.113.5', bot_detector_verified_ip($spoofed, []), 'collected signals'); + TestRunner::assertSame('203.0.113.5', bot_detector_verified_ip($spoofed, [], bot_detector_signals_without_ip()), 'missing ip_address'); +}); + +TestRunner::test('a client that is not a trusted proxy cannot spoof past one configured elsewhere', function () use ($spoofed) { + TestRunner::assertSame('203.0.113.5', bot_detector_verified_ip($spoofed, ['10.0.0.0/8'], bot_detector_signals_without_ip())); +}); + +TestRunner::test('behind a trusted proxy the X-Forwarded-For chain is read right to left', function () { + $server = ['REMOTE_ADDR' => '10.0.0.5', 'HTTP_X_FORWARDED_FOR' => '192.0.2.77, 198.51.100.7, 10.0.0.9']; + TestRunner::assertSame('198.51.100.7', bot_detector_verified_ip($server, ['10.0.0.0/8'], bot_detector_signals_without_ip())); +}); + +TestRunner::test('IPv6 chains resolve the same way', function () { + $server = ['REMOTE_ADDR' => '2001:db8:ffff::1', 'HTTP_X_FORWARDED_FOR' => '2001:db8::bad, 2001:db8:1::7, 2001:db8:ffff::2']; + TestRunner::assertSame('2001:db8:1::7', bot_detector_verified_ip($server, ['2001:db8:ffff::/48'], bot_detector_signals_without_ip()), 'trusted IPv6 proxy'); + $direct = ['REMOTE_ADDR' => '2001:db8:2::5', 'HTTP_X_FORWARDED_FOR' => '2001:db8::bad']; + TestRunner::assertSame('2001:db8:2::5', bot_detector_verified_ip($direct, [], bot_detector_signals_without_ip()), 'untrusted IPv6 peer'); +}); + +TestRunner::test('a valid supplied ip_address is kept, an invalid one is resolved', function () use ($spoofed) { + $signals = bot_detector_signals_without_ip(); + TestRunner::assertSame('198.51.100.9', bot_detector_verified_ip($spoofed, [], $signals + ['ip_address' => '198.51.100.9'])); + TestRunner::assertSame('203.0.113.5', bot_detector_verified_ip($spoofed, [], $signals + ['ip_address' => 'not-an-ip'])); +}); + +TestRunner::test('BotDetector and SignalCollector resolve the same request identically', function () { + $cases = [ + [['REMOTE_ADDR' => '203.0.113.5', 'HTTP_X_FORWARDED_FOR' => '192.0.2.77'], []], + [['REMOTE_ADDR' => '10.0.0.5', 'HTTP_X_FORWARDED_FOR' => '192.0.2.77, 198.51.100.7'], ['10.0.0.0/8']], + [['REMOTE_ADDR' => '10.0.0.5', 'HTTP_CF_CONNECTING_IP' => '198.51.100.8'], ['10.0.0.0/8']], + [['REMOTE_ADDR' => '2001:db8:ffff::1', 'HTTP_X_FORWARDED_FOR' => '2001:db8:1::7'], ['2001:db8:ffff::/48']], + ]; + foreach ($cases as [$server, $proxies]) { + $saved = $_SERVER; + $_SERVER = $server; + $expected = (new \WebDecoy\SignalCollector($proxies))->getIP(); + $_SERVER = $saved; + TestRunner::assertSame($expected, bot_detector_verified_ip($server, $proxies, bot_detector_signals_without_ip()), $server['REMOTE_ADDR']); + } +}); + +TestRunner::test('the WordPress detector wrapper uses the configured trusted proxies', function () { + $GLOBALS['wd_bot_detector_ip_proxies'] = ['10.0.0.0/8']; + if (!function_exists('webdecoy_plugin_trusted_proxies')) { + function webdecoy_plugin_trusted_proxies(): array + { + return $GLOBALS['wd_bot_detector_ip_proxies'] ?? []; + } + } + $saved = $_SERVER; + $_SERVER = ['REMOTE_ADDR' => '10.0.0.5', 'HTTP_X_FORWARDED_FOR' => '198.51.100.7']; + try { + $wrapper = new WebDecoy_Detector([]); + $prop = new ReflectionProperty(WebDecoy_Detector::class, 'detector'); + if (PHP_VERSION_ID < 80100) { + $prop->setAccessible(true); + } + TestRunner::assertSame('198.51.100.7', $prop->getValue($wrapper)->getSignalCollector()->getIP()); + } finally { + $_SERVER = $saved; + unset($GLOBALS['wd_bot_detector_ip_proxies']); + } +});