Use composer/pcre for better regexp static analysis (#416)

Use composer/preg's Regex class instead of `preg_*()` and `Strings::*()`. Also use the provided PHPStan extension to check regular expressions. Close #401
spaze · Oct 20, 2024 · 00f510c · 00f510c
2 parents e4c47e7 + a9a92be
commit 00f510c
Show file tree

Hide file tree

Showing 43 changed files with 2,191 additions and 34 deletions.
diff --git a/app/composer.json b/app/composer.json
@@ -17,6 +17,7 @@
 		"ext-pdo": "*",
 		"ext-simplexml": "*",
 		"ext-sodium": "*",
+		"composer/pcre": "^3.3.1",
 		"contributte/translation": "^2.0",
 		"latte/latte": "^3.0.3",
 		"nette/application": "^3.1.10",

diff --git a/app/composer.lock b/app/composer.lock
diff --git a/app/disallowed-calls.neon b/app/disallowed-calls.neon
@@ -15,12 +15,21 @@ parameters:
 		-
 			function: 'setcookie()'
 			message: 'use methods from MichalSpacekCz\Http\Cookies'
+		-
+			function: 'preg_*()'
+			message: 'use the Preg class from composer/pcre'
 	disallowedStaticCalls:
 		-
 			method: 'Tester\Environment::skip()'
 			message: 'use TestCaseRunner::skip() instead, it can ignore skipping with an environment variable'
 			allowInMethods:
 				- 'MichalSpacekCz\Test\TestCaseRunner::skip()'
+		-
+			method:
+				- 'Nette\Utils\Strings::match()'
+				- 'Nette\Utils\Strings::matchAll()'
+				- 'Nette\Utils\Strings::replace()'
+			message: 'use the Preg or Regex class from composer/pcre for better static analysis'
 	disallowedMethodCalls:
 		-
 			method:

diff --git a/app/phpstan-vendor.neon b/app/phpstan-vendor.neon
@@ -32,6 +32,7 @@ parameters:
 			- vendor/slevomat/coding-standard/*
 			- vendor/squizlabs/php_codesniffer/*
 			# Throws "No error to ignore is reported on line", which can't be ignored, probably because only custom ruleset is used
+			- vendor/composer/pcre/src/Regex.php
 			- vendor/contributte/translation/src/DI/TranslationExtension.php
 			- vendor/contributte/translation/src/Latte/Macros.php
 			- vendor/metisfw/phpstan-nette-links/src/Nette/PresenterResolver.php
@@ -58,6 +59,10 @@ parameters:
 				- vendor/latte/latte/src/Tools/Linter.php
 				- vendor/nette/tester/src/Runner/CliTester.php
 				- vendor/nette/tester/src/Runner/Runner.php
+		-
+			function: 'preg_*()'
+			allowIn:
+				- vendor/*.php
 		# bundled disallowed-dangerous-calls.neon
 		-
 			function: 'eval()'
@@ -208,6 +213,14 @@ parameters:
 			message: 'use TestCaseRunner::skip() instead, it can ignore skipping with an environment variable'
 			allowIn:
 				- vendor/nette/tester/src/Framework/TestCase.php
+		-
+			method:
+				- 'Nette\Utils\Strings::match()'
+				- 'Nette\Utils\Strings::matchAll()'
+				- 'Nette\Utils\Strings::replace()'
+			allowIn:
+				- vendor/contributte/*.php
+				- vendor/nette/*.php
 	disallowedSuperglobals:
 		-
 			superglobal: '$_SERVER'

diff --git a/app/phpstan.neon b/app/phpstan.neon
@@ -22,3 +22,4 @@ includes:
 	- vendor/phpstan/phpstan-deprecation-rules/rules.neon
 	- vendor/metisfw/phpstan-nette-links/extension.neon
 	- vendor/metisfw/phpstan-nette-links/rules.neon
+	- vendor/composer/pcre/extension.neon
diff --git a/app/psalm-baseline.xml b/app/psalm-baseline.xml
@@ -32,11 +32,6 @@
       <code><![CDATA[$units->id]]></code>
     </MixedArgument>
   </file>
-  <file src="src/EasterEgg/CrLfUrlInjections.php">
-    <MixedArgument>
-      <code><![CDATA[$match[1]]]></code>
-    </MixedArgument>
-  </file>
   <file src="src/EasterEgg/WinterIsComing.php">
     <MixedArgument>
       <code><![CDATA[$input->getValue()]]></code>

diff --git a/app/src/EasterEgg/CrLfUrlInjections.php b/app/src/EasterEgg/CrLfUrlInjections.php
@@ -3,9 +3,9 @@
 
 namespace MichalSpacekCz\EasterEgg;
 
+use Composer\Pcre\Regex;
 use Nette\Http\IRequest;
 use Nette\Http\IResponse;
-use Nette\Utils\Strings;
 
 readonly class CrLfUrlInjections
 {
@@ -26,12 +26,12 @@ public function detectAttempt(): bool
 		if (!str_contains($url, "\r") && !str_contains($url, "\n")) {
 			return false;
 		}
-		$matches = Strings::matchAll($url, sprintf('/Set\-Cookie:%s=([a-z0-9]+)/i', self::COOKIE_NAME));
-		foreach ($matches as $match) {
+		$matches = Regex::matchAllStrictGroups(sprintf('/Set\-Cookie:%s=([a-z0-9]+)/i', self::COOKIE_NAME), $url);
+		foreach ($matches->matches[1] as $match) {
 			// Don't use any cookie name from the request to avoid e.g. session fixation
 			$this->httpResponse->setCookie(
 				self::COOKIE_NAME,
-				$match[1],
+				$match,
 				time() - 31337 * 1337,
 				'/expired=31337*1337seconds/(1.3years)/ago',
 			);

diff --git a/app/src/EasterEgg/WinterIsComing.php b/app/src/EasterEgg/WinterIsComing.php
@@ -3,12 +3,12 @@
 
 namespace MichalSpacekCz\EasterEgg;
 
+use Composer\Pcre\Regex;
 use MichalSpacekCz\ShouldNotHappenException;
 use Nette\Application\Responses\TextResponse;
 use Nette\Application\UI\Presenter;
 use Nette\Forms\Controls\TextInput;
 use Nette\Utils\Arrays;
-use Nette\Utils\Strings;
 
 class WinterIsComing
 {
@@ -42,7 +42,7 @@ public function ruleEmail(): callable
 				is_string($input->getValue())
 				&& (
 					Arrays::contains(self::EMAILS, $input->getValue())
-					|| Strings::match($input->getValue(), '/@(' . implode('|', array_map('preg_quote', self::HOSTS)) . ')$/') !== null
+					|| Regex::isMatch('/@(' . implode('|', array_map('preg_quote', self::HOSTS)) . ')$/', $input->getValue())
 				)
 			) {
 				$this->sendSyntaxError($input);

diff --git a/app/src/Form/SignInHoneypotFormFactory.php b/app/src/Form/SignInHoneypotFormFactory.php
@@ -3,6 +3,7 @@
 
 namespace MichalSpacekCz\Form;
 
+use Composer\Pcre\Regex;
 use MichalSpacekCz\Form\Controls\FormControlsFactory;
 use Nette\Http\IRequest;
 use Nette\Utils\Html;
@@ -27,7 +28,7 @@ public function create(): UiForm
 			$values = $form->getFormValues();
 			Debugger::log("Sign-in attempt: {$values->username}, {$values->password}, {$this->httpRequest->getRemoteAddress()}", 'honeypot');
 			$creds = $values->username . ':' . $values->password;
-			if (preg_match('~\slimit\s~i', $creds)) {
+			if (Regex::isMatch('~\slimit\s~i', $creds)) {
 				$message = Html::el()
 					->setText("No, no, no, no, no, no, no, no, no, no, no, no there's ")
 					->addHtml(Html::el('a')
@@ -37,7 +38,7 @@ public function create(): UiForm
 					->addText('!');
 			} elseif (stripos($creds, 'honeypot') !== false) {
 				$message = 'Jo jo, honeypot, přesně tak';
-			} elseif (preg_match('~\sor\s~i', $creds)) {
+			} elseif (Regex::isMatch('~\sor\s~i', $creds)) {
 				$message = 'Dobrej pokusql!';
 			} else {
 				$message = 'Špatné uživatelské jméno nebo heslo';

diff --git a/app/src/Formatter/TexyFormatter.php b/app/src/Formatter/TexyFormatter.php
@@ -3,12 +3,12 @@
 
 namespace MichalSpacekCz\Formatter;
 
+use Composer\Pcre\Regex;
 use Contributte\Translation\Exceptions\InvalidArgument;
 use Contributte\Translation\Translator;
 use MichalSpacekCz\Formatter\Placeholders\TexyFormatterPlaceholder;
 use MichalSpacekCz\Utils\Hash;
 use Nette\Utils\Html;
-use Nette\Utils\Strings;
 use Stringable;
 use Symfony\Contracts\Cache\CacheInterface;
 use Symfony\Contracts\Cache\ItemInterface;
@@ -151,7 +151,7 @@ public function format(string $text): Html
 	{
 		$texy = $this->texy ?? $this->getTexy();
 		return $this->replace($text . self::CACHE_KEY_DELIMITER . __FUNCTION__, $texy, function () use ($texy, $text): string {
-			return Strings::replace($texy->process($text), '~^\s*<p[^>]*>(.*)</p>\s*$~s', '$1');
+			return Regex::replace('~^\s*<p[^>]*>(.*)</p>\s*$~s', '$1', $texy->process($text))->result;
 		});
 	}
 
@@ -188,13 +188,13 @@ private function replace(string $key, Texy $texy, callable $callback): Html
 			$replacements[$placeholder::getId()] = $placeholder->replace(...);
 		}
 
-		$result = Strings::replace(
-			$result,
+		$result = Regex::replaceCallbackStrictGroups(
 			'~\*\*([^:]+):([^*]+)\*\*~',
 			function (array $matches) use ($replacements): string {
 				return (isset($replacements[$matches[1]]) ? $replacements[$matches[1]]($matches[2]) : '');
 			},
-		);
+			$result,
+		)->result;
 		return Html::el()->setHtml($result);
 	}
 

diff --git a/app/src/Formatter/TexyPhraseHandler.php b/app/src/Formatter/TexyPhraseHandler.php
@@ -3,6 +3,7 @@
 
 namespace MichalSpacekCz\Formatter;
 
+use Composer\Pcre\Regex;
 use Contributte\Translation\Translator;
 use MichalSpacekCz\Application\Locale\LocaleLinkGenerator;
 use MichalSpacekCz\Articles\Blog\BlogPostLocaleUrls;
@@ -61,8 +62,11 @@ public function solve(HandlerInvocation $invocation, string $phrase, string $con
 		}
 
 		// "title":[link-en_US:Module:Presenter:action params]
-		if (str_starts_with($url, 'link-') && preg_match("/^link-{$localeRegExp}:(.*)\\z/", $url, $matches)) {
-			$link->URL = $this->getLink($matches[2], $matches[1]);
+		if (str_starts_with($url, 'link-')) {
+			$result = Regex::matchStrictGroups("/^link-{$localeRegExp}:(.*)\\z/", $url);
+			if ($result->matched) {
+				$link->URL = $this->getLink($result->matches[2], $result->matches[1]);
+			}
 		}
 
 		// "title":[blog:post#fragment]
@@ -71,8 +75,11 @@ public function solve(HandlerInvocation $invocation, string $phrase, string $con
 		}
 
 		// "title":[blog-en_US:post#fragment]
-		if (str_starts_with($url, 'blog-') && preg_match("/^blog-{$localeRegExp}:(.*)\\z/", $url, $matches)) {
-			$link->URL = $this->getBlogLink($matches[2], $matches[1]);
+		if (str_starts_with($url, 'blog-')) {
+			$result = Regex::matchStrictGroups("/^blog-{$localeRegExp}:(.*)\\z/", $url);
+			if ($result->matched) {
+				$link->URL = $this->getBlogLink($result->matches[2], $result->matches[1]);
+			}
 		}
 
 		// "title":[inhouse-training:training]

diff --git a/app/src/Training/ApplicationForm/TrainingApplicationFormSpam.php b/app/src/Training/ApplicationForm/TrainingApplicationFormSpam.php
@@ -3,6 +3,7 @@
 
 namespace MichalSpacekCz\Training\ApplicationForm;
 
+use Composer\Pcre\Regex;
 use MichalSpacekCz\Training\Exceptions\SpammyApplicationException;
 use stdClass;
 
@@ -17,7 +18,7 @@ class TrainingApplicationFormSpam
 
 	public function check(stdClass $values): void
 	{
-		if (preg_match('~\s+href="\s*https?://~', $values->note ?? '')) {
+		if (Regex::isMatch('~\s+href="\s*https?://~', $values->note ?? '')) {
 			throw new SpammyApplicationException();
 		} elseif (
 			ctype_lower($values->name ?? self::FIELD_MISSING_VALUE)

diff --git a/app/src/Training/Applications/TrainingApplicationSources.php b/app/src/Training/Applications/TrainingApplicationSources.php
@@ -3,6 +3,7 @@
 
 namespace MichalSpacekCz\Training\Applications;
 
+use Composer\Pcre\Regex;
 use MichalSpacekCz\Database\TypedDatabase;
 use MichalSpacekCz\Training\Resolver\Vrana;
 use Nette\Utils\Strings;
@@ -60,9 +61,9 @@ public function getDefaultSource(): string
 	 */
 	public function getSourceNameInitials(string $name): string
 	{
-		$name = Strings::replace($name, '/,? s\.r\.o./', '');
-		$matches = Strings::matchAll($name, '/(?<=\s|\b)\pL/u', PREG_PATTERN_ORDER);
-		return Strings::upper(implode('', current($matches)));
+		$replace = Regex::replace('/,? s\.r\.o./', '', $name);
+		$matches = Regex::matchAll('/(?<=\s|\b)\pL/u', $replace->result);
+		return Strings::upper(implode('', $matches->matches[0]));
 	}
 
 }
diff --git a/app/src/UpcKeys/Technicolor.php b/app/src/UpcKeys/Technicolor.php
@@ -3,6 +3,7 @@
 
 namespace MichalSpacekCz\UpcKeys;
 
+use Composer\Pcre\Regex;
 use DateTime;
 use MichalSpacekCz\Http\Client\HttpClient;
 use MichalSpacekCz\Http\Client\HttpClientRequest;
@@ -95,10 +96,11 @@ private function generateKeys(string $ssid): array
 				continue;
 			}
 
-			if (!preg_match('/([^,]+),([^,]+),(\d+)/', $line, $matches)) {
+			$result = Regex::matchStrictGroups('/([^,]+),([^,]+),(\d+)/', $line);
+			if (!$result->matched) {
 				throw new UpcKeysApiIncorrectTokensException($json, $line);
 			}
-			[, $serial, $key, $type] = $matches;
+			[, $serial, $key, $type] = $result->matches;
 			$keys["{$type}-{$serial}"] = $this->buildKey($serial, $key, (int)$type);
 		}
 		ksort($keys);
@@ -186,8 +188,7 @@ private function storeKeys(string $ssid, array $keys): void
 	 */
 	private function buildKey(string $serial, string $key, int $type): WiFiKey
 	{
-		preg_match('/^[a-z]+/i', $serial, $matches);
-		$prefix = current($matches);
+		$prefix = Regex::match('/^[a-z]+/i', $serial)->matches[0] ?? false;
 		if ($prefix === false || !in_array($prefix, self::PREFIXES)) {
 			throw new UpcKeysApiUnknownPrefixException($serial);
 		}