Trivadis / plsql-cop-cli

db* CODECOP Command Line
Other
24 stars 1 forks source link

False positive for G-6020 when dynamic SQL is not a INSERT, UPDATE or DELETE statement #16

Closed PhilippSalvisberg closed 5 months ago

PhilippSalvisberg commented 6 months ago

A violation of G-6020 is reported in the following case:

declare
   co_sql constant user_tab_comments.comment%type := 'begin :ret := 100; end;';
   l_ret  integer;
begin
   execute immediate co_sql using out l_ret; -- G-6020 false positive
   sys.dbms_output.put_line(l_ret);
end;
/

The returning clause cannot be used in this case.

To reduce false positives it could be an option to scan the statement for insert, update, and delete. Only if one of these words is found a violation should be thrown. In this case, a returning clause should be applicable. However, even then false positives are possible, e.g. when a dynamic PL/SQL block contains an insert statement, but it is less likely. A side effect of this approach is, that there will be false negatives, for example when the statement to be executed cannot be evaluated. Trying to find out if the statement contains such a keyword can be costly and should be done only when an out parameter is defined.

In any case, this is a limitation and should be documented.

PhilippSalvisberg commented 5 months ago

The returning clause is not applicable in select and merge statements and in PL/SQL blocks. Using an out parameter in a select statement does not work. So it might be better to search for begin. This way we avoid false positives when DML is used in the dynamic PL/SQL block.

We could handle dynamic PL/SQL blocks as an exception. However, this could still lead to false positives when the PL/SQL block is unavailable in the code. For example, when passed via a parameter or read from a table.

False positives nor false negatives are avoidable. Still, I think we get fewer false positives when looking for update, insert and delete.

PhilippSalvisberg commented 5 months ago

fixed with Azure DevOps commit