[ADD] eval-referenced: Detects if a "eval" is referenced (without call it)

[JesusZapata]

Detects referenced and inferred cases

def func2(param): 
       param("c = 2")

 def func3():
     my_dict = {
         'my_eval': eval,  # [eval-referenced]
     }
     my_list = [eval]  # [eval-referenced]
 
     my_var = eval  # [eval-referenced]
     # inferred case
     my_var('d = 3')  # [eval-referenced]
     func2(eval)  # [eval-referenced]
     return my_dict, my_list

[moylop260]

The history of this check is funny.

I reviewed a old code because a translation wasn't loaded.
Then I found a context used from report like as:

context_used_in_report = {
   'eval': eval,
}

You know, the report is a template data loaded directly in the database.
Then a user could change the template and run any eval sentence!

The big vulnerability was injected 😢

pylint native detects just eval() invoke but not eval assignation or referenced.

Really this case is very dangerous and so hard to detect visually.
Then this check born thanks to that case.

FYI maintainer of pylint don't like this check pylint-dev/pylint#1215
because they are using a local template with eval from dict (Without database for users) then in that context is not dangerous, but for odoo is other case.

@lasley @pedrobaeza

[pedrobaeza]

OK, I see the potential problem, and indeed we can detect this. In Odoo modules, no eval should be used (only safe_eval).

[lasley]

Yeah I think this is a good idea @JesusZapata & @moylop260 - thank you.

I see where the maintainers of PyLint are coming from, but here in Odoo land it is like @pedrobaeza says - there is never a reason in which we should not be using safe_eval

[lasley]

Can we name these methods something a little more descriptive? I know they're test methods, but func2 and func3 really hold no context on their own and require that you read the methods, or at least the doc block, to understand what they are doing.

[moylop260]

This was extracted from original test of pylint.
Here we can fails all good practices.
But we dont have problem to change it
What is a good name for you in this case?

[lasley]

Ummm maybe something similar to the docblocks? eval_from_param and eval_from_other?

[lasley]

Thansk @JesusZapata

[JesusZapata]

@lasley Thanks

[moylop260]

@JesusZapata
After merge this one you should create a new PR to enable this new check from MQT

[JesusZapata]

@moylop260 Done! OCA/maintainer-quality-tools#403