Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions lib/CleantalkSP/SpbctWP/Firewall/WAF.php
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,11 @@ private static function hasSignature($data, $signature)
$result_signature_body = false;
$result_additionally_params = false;

// Skip catch-all regex without headers/URL (e.g. #.*# with empty waf_headers) — mass FP
if ( self::isUnsafeCatchAllSignature($signature_body, $what_to_check_additionally) ) {
return false;
}

$is_regexp = Helper::isRegexp($signature_body);

if ( $is_regexp && preg_match($signature_body, $data) ) {
Expand Down Expand Up @@ -456,6 +461,33 @@ private static function hasSignature($data, $signature)
return false;
}

/**
* Detect catch-all regex WAF rules that have no headers/URL constraints.
* Such rules (e.g. body "#.*#" with empty waf_headers) match every request
* and cause mass false positives.
*
* @param string $signature_body
* @param string $what_to_check_additionally
*
* @return bool
*/
private static function isUnsafeCatchAllSignature($signature_body, $what_to_check_additionally)
{
if ( $what_to_check_additionally !== 'nothing' ) {
return false;
}

if ( ! is_string($signature_body) || ! Helper::isRegexp($signature_body) ) {
return false;
}

// Matches empty string and an arbitrary probe → pattern accepts any input
$probe = 'spbct_waf_probe_catchall_check';

return @preg_match($signature_body, '') === 1
&& @preg_match($signature_body, $probe) === 1;
}

/**
* Compare signature headers and request headers
*
Expand Down
183 changes: 183 additions & 0 deletions tests/lib/CleantalkSP/SpbctWP/Firewall/WAFHasSignatureTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
<?php

namespace CleantalkSP\SpbctWP\Firewall;

use CleantalkSP\Common\Helpers\HTTP;
use ReflectionMethod;

class WAFHasSignatureTest extends \SpbcTestCase
{
/**
* @var ReflectionMethod
*/
private $hasSignature;

/**
* @var ReflectionMethod
*/
private $isUnsafeCatchAllSignature;

protected function setUp(): void
{
parent::setUp();

$this->hasSignature = new ReflectionMethod(WAF::class, 'hasSignature');
$this->hasSignature->setAccessible(true);

$this->isUnsafeCatchAllSignature = new ReflectionMethod(WAF::class, 'isUnsafeCatchAllSignature');
$this->isUnsafeCatchAllSignature->setAccessible(true);
Comment thread
svfcode marked this conversation as resolved.

// Reset cached HTTP headers between tests
HTTP::getInstance()->http_headers = array();
}

protected function tearDown(): void
{
HTTP::getInstance()->http_headers = array();
unset($_SERVER['HTTP_USER_AGENT']);
parent::tearDown();
}

/**
* @param string $data
* @param array $signature
*
* @return bool
*/
private function callHasSignature($data, array $signature)
{
return $this->hasSignature->invoke(null, $data, $signature);
}

/**
* Incident #4471: catch-all body without headers must not match.
*/
public function testCatchAllWithoutHeadersDoesNotMatch()
{
$signature = array(
'body' => '#.*#',
'waf_headers' => null,
'waf_url' => null,
);

$this->assertFalse($this->callHasSignature('', $signature));
$this->assertFalse($this->callHasSignature('anything', $signature));
$this->assertFalse($this->callHasSignature('?page=1', $signature));
}

/**
* Empty string headers are treated the same as NULL.
*/
public function testCatchAllWithEmptyHeadersDoesNotMatch()
{
$signature = array(
'body' => '#.*#',
'waf_headers' => '',
'waf_url' => '',
);

$this->assertFalse($this->callHasSignature('query', $signature));
}

/**
* Catch-all with modifiers must also be rejected without constraints.
*/
public function testCatchAllWithModifiersWithoutHeadersDoesNotMatch()
{
$signature = array(
'body' => '#.*#i',
'waf_headers' => null,
'waf_url' => null,
);

$this->assertFalse($this->callHasSignature('query', $signature));
}

/**
* Catch-all is valid when constrained by matching request headers.
*/
public function testCatchAllWithMatchingHeadersDoesMatch()
{
$_SERVER['HTTP_USER_AGENT'] = 'WPSCAN';
HTTP::getInstance()->http_headers = array();

$signature = array(
'body' => '#.*#',
'waf_headers' => json_encode(array('User-Agent' => 'WPSCAN')),
'waf_url' => null,
);

$this->assertTrue($this->callHasSignature('', $signature));
$this->assertTrue($this->callHasSignature('any-query', $signature));
}

/**
* Catch-all with headers that do not match the request must not fire.
*/
public function testCatchAllWithNonMatchingHeadersDoesNotMatch()
{
$_SERVER['HTTP_USER_AGENT'] = 'Mozilla/5.0';
HTTP::getInstance()->http_headers = array();

$signature = array(
'body' => '#.*#',
'waf_headers' => json_encode(array('User-Agent' => 'WPSCAN')),
'waf_url' => null,
);

$this->assertFalse($this->callHasSignature('any-query', $signature));
}

/**
* Narrow regexp without headers must still work (not a catch-all).
*/
public function testNarrowRegexpWithoutHeadersStillMatches()
{
$signature = array(
'body' => '#wp-config\.php#',
'waf_headers' => null,
'waf_url' => null,
);

$this->assertTrue($this->callHasSignature('download=wp-config.php', $signature));
$this->assertFalse($this->callHasSignature('page=home', $signature));
$this->assertFalse($this->callHasSignature('', $signature));
}

/**
* Literal (non-regexp) body without headers must still work.
*/
public function testLiteralBodyWithoutHeadersStillMatches()
{
$signature = array(
'body' => 'eval(',
'waf_headers' => null,
'waf_url' => null,
);

$this->assertTrue($this->callHasSignature('x=eval(base64_decode(1))', $signature));
$this->assertFalse($this->callHasSignature('x=safe', $signature));
}

/**
* Direct unit coverage for the catch-all detector.
*/
public function testIsUnsafeCatchAllSignatureDetector()
{
$this->assertTrue(
$this->isUnsafeCatchAllSignature->invoke(null, '#.*#', 'nothing')
);
$this->assertTrue(
$this->isUnsafeCatchAllSignature->invoke(null, '#^.*$#', 'nothing')
);
$this->assertFalse(
$this->isUnsafeCatchAllSignature->invoke(null, '#.*#', 'only_headers')
);
$this->assertFalse(
$this->isUnsafeCatchAllSignature->invoke(null, '#wp-config#', 'nothing')
);
$this->assertFalse(
$this->isUnsafeCatchAllSignature->invoke(null, 'eval(', 'nothing')
);
}
}