Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive PH2 error #181

Closed
TLWebdesign opened this issue Feb 10, 2022 · 4 comments
Closed

False positive PH2 error #181

TLWebdesign opened this issue Feb 10, 2022 · 4 comments

Comments

@TLWebdesign
Copy link

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
Copy link
Collaborator

dryabov commented Feb 11, 2022

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
Copy link
Collaborator

Llewellynvdm commented Feb 12, 2022

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
Copy link
Collaborator

dryabov commented Feb 13, 2022

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
Copy link
Collaborator

dryabov commented Feb 13, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants