joomla-extensions / jedchecker

Joomla extension to check components, modules or plugins for possible problems for submission to the JED -> Translations: https://joomla.crowdin.com/joomla-official-extensions
38 stars 28 forks source link

False positive PH2 error #181

Closed TLWebdesign closed 11 months ago

TLWebdesign commented 2 years ago

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

dryabov commented 2 years ago

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.

Llewellynvdm commented 2 years ago

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.

dryabov commented 2 years ago

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 commented 2 years ago

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