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: Pattern found#24 - PHP execution operator: backticks (``) #159

Closed anibalsanchez closed 2 years ago

anibalsanchez commented 2 years ago

Hi,

This piece of code fired the rule:

    /**
     * Подсчитывает количество товаров в массиве
     * @return int
     *
     * @since 1.0.0
     */
    public function getTotal() : int
    {
        if (!empty($this->products))
        {
            return count($this->products);
        } else{
            return 0;
        }
    }

@dryabov

dryabov commented 2 years ago

The problem is in very simple regex in JAMSS (\$\w[^;=\)]*=[^;=\)]*`.*`), unlikely it may be fixed easily. The proper way is to use token_get_all and search for ` token there, but it's out of JAMSS approach.

Llewellynvdm commented 2 years ago

The sample code does not seem to have the ` or what am I missing...

anibalsanchez commented 2 years ago

@dryabov A new case related to backticks and images:

#001 /joomhelper360/assets/css/ajax-loader.gif in line: 26
? Pattern found#24 - PHP execution operator: backticks (``)
dryabov commented 2 years ago

I know, there is a lot of false-positives with this rule, because it just finds $ followed by = and two `s (at any distances). That's why the warning message may contain a code that doesn't contain ` at all, because of it may be located many lines below (usually inside of a quoted string).

Llewellynvdm commented 2 years ago

in that case, just giving the file path and not the code will be more helpful... so that we do self investigation. Since seeing the code, and not seeing the issue make one ignore it, and that is bad.

dryabov commented 2 years ago

My current idea is to implement a concept of "scopes" for JAMSS rules, e.g. the check of PHP embedded into a GIF file requires to analyze the entire file (a "full" scope), and this check for backticks requires to analyze PHP code only (a "code" scope, i.e. excluding HTML and quoted strings). As a result, most of false-positives will be eliminated.