Closed npr-gh closed 4 months ago
Based on the information you provide this seems like a very contrived example. You suggest it is a potential vector for a denial-of-service attack but only in the situation where:
This seems like a totally unrealistic example and nowhere near the normal use case for PHPCodeSniffer. Why are you feeding CSS into a tool for PHP?
@npr-gh @martinjoiner Well, PHPCS does currently still handle CSS and JS (up to a point), but the PHPCompatibility rulesets never have, nor will.
As per the readme on those projects: use --extensions=php
when running the PHPCompatibility rulesets.
Or if combined with other rulesets, use the below in a custom ruleset to limit PHPCompatibility to PHP files:
<rule ref="PHPCompatibilityWP">
<include-pattern>*\.php$</include-pattern>
</rule>
(though the problem is likely to be in the CSS tokenizer, but as that will be removed in v4, this is unlikely to be fixed)
On another note: don't open issues in this repo anymore, use the new repo instead. See #3932.
Describe the bug
PHPSniffer managed to run forever analyzing a certain CSS file. (I discovered later, 'practically forever', as in longer than the heat death of the universe, but still it theoretically would complete in a finite time).
After the verbose mode created a 17-gigabyte log, I cut it off.
The section of the CSS file that causes the problem is this bit of code.
The issue is a very gross inefficiency on the part of PHPSniffer, it seems to scale as O(2^n).
For each line of "#available_widgets", it appears that the entire file is scanned again, recursively. This creates an exponential amount of work.
To reproduce
Run the following on the command line:
Expected behaviour
Actual behaviour:
A little interesting experiment that really exposes the behaviour is to take the initial block of CSS only, then add the
#available-widgets
lines block by block. I started at a random position in the middle/* dashicons-admin-users */
, and added blocks one by one, then looked at the execution time. I didn't do a lot of sampling so there's quite a bit of statistical error here. The exponential behaviour is pretty clear after a few iterations of adding more lines to the file.t ~ 23.383e(0.325L) where t = time in ms, L = number of '#available-widgets' lines is a least-squares exponential fit to this data.
Versions (please complete the following information)
Additional context
This can cause the viability of Denial-of-service attacks if PHPCodeSniffer / PHPCompatibilityWP is provided as a service and may be a security issue in this context.
Please confirm:
master
branch of PHP_CodeSniffer.