jedchecker icon indicating copy to clipboard operation
jedchecker copied to clipboard

False positive PH2 error

Open TLWebdesign opened this issue 3 years ago • 4 comments

Hi,

I have checked an extension and it got a false positive on the PH2 error.

It contained this code as the start of the file:

<?php
<?php
/**
 * @author ****
 * @copyright 2016
 * @version  2.0 (10-01-2022)
 * @license   <a href="http://www.gnu.org/licenses/gpl-3.0.html" target="_blank">GNU/GPLv3</a>
 * 
 */
defined("_JEXEC") or die("Restricted access");

It caught the error as in that it got double php opening tags but it listed it as PH2 error.

Kind regards,

Tom

TLWebdesign avatar Feb 10 '22 22:02 TLWebdesign

PH2 rule does more than just a check defined("_JEXEC") exists in the code. It ensures that this check is the first executed statement in the code (and it's the main reason for this check: to prevent any code execution in non-Joomla context to avoid possible path disclosure and other vulnerabilities).

PHP reads your code as T_OPEN_TAG token ("<?php") followed by the "<" comparison operator (resulting in the "Parse error" here), and so this rule is triggered.

Maybe next releases of the JED Checker will check PHP syntax as well and detect such issues automatically, but I'm very glad to know this error (duplicated <?php tag) is indirectly found by the current version of JED Checker.

dryabov avatar Feb 11 '22 11:02 dryabov

Okay this makes sense and yes doing some PHP syntax validation would be helpful, what approach do you have in mind?

What we also need, to detect depreciated methods, classes and method signature mismatching of the Joomla API, this will a be a huge step-up... and something I would really want to help happen.

Llewellynvdm avatar Feb 12 '22 22:02 Llewellynvdm

what approach do you have in mind?

https://github.com/nikic/PHP-Parser, but it requires PHP7.0+ (note that currently required PHP version in JED Checker is 5.6). It's pretty easy to implement, just a simple try/catch block.

What we also need, to detect depreciated methods, classes and method signature mismatching of the Joomla API

It's quite complicated, because of dynamical nature of PHP, so any code analyzer cannot be sure about the type of a given variable. Probably, the best solution here is to use https://github.com/ircmaxell/php-cfg to track variables lifetimes, but it has some known issues (doesn't take into account parameters passed by reference and doesn't process try/catch/finally blocks properly).

An alternative is to use PHPStan with https://github.com/phpstan/phpstan-deprecation-rules, but it's quite a large project (PHPStan is shipped as a 20Mb phpstan.phar file). And I'm not sure it is able to cover nontrivial cases.

dryabov avatar Feb 13 '22 06:02 dryabov

One more tool to found deprecated methods: https://github.com/vimeo/psalm (size of phar is 11Mb only).

dryabov avatar Feb 13 '22 13:02 dryabov