phpcs-security-audit icon indicating copy to clipboard operation
phpcs-security-audit copied to clipboard

Skip validating user defined functions which has similar name as file system function

Open mgalang opened this issue 5 years ago • 2 comments

Description

The Filesystem function sniff is also detecting and validating user defined functions even if the function is not a PHP native filesystem function.

Proposed fixed: Check if previous token is "function" to check if the function is user defined.

Steps to reproduce:

  1. Create a function called delete function delete($myvalue) {}
  2. Run phpcs
  3. It throws a warning Filesystem function delete() detected with dynamic parameter

mgalang avatar Mar 02 '20 13:03 mgalang

I think that this is a valid way of diminishing false positives.

But why just limit it to filesystem functions? I can see this one giving false positive too:

function exec($myvalue) {}

I'm just assuming you're fixing this one because delete is a common function that is being used in PHP but I want to explore pushing your idea further to have a broader impact.

Side note on php tokens; We should compare using "T_FUNCTION" on ['code'] instead of "function" on ['content'].

jmarcil avatar Mar 16 '20 03:03 jmarcil

@jmarcil FYI: PHPCSUtils will have an abstract sniff for function call sniffing which takes this and much more (aliased functions in use statements, method calls etc) into account and would greatly reduce false positives for all sniffs looking at (global) function calls.

It might be worth considering adding that as a dependency.

I expect that abstract will be pulled within the next few weeks.

jrfnl avatar Mar 16 '20 03:03 jrfnl