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 for G-6020 when dynamic SQL is not a INSERT, UPDATE or DELETE statement #16

Closed
PhilippSalvisberg opened this issue Dec 21, 2023 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@PhilippSalvisberg
Copy link
Collaborator

PhilippSalvisberg commented Dec 21, 2023

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 PhilippSalvisberg self-assigned this Dec 21, 2023
@PhilippSalvisberg PhilippSalvisberg added the bug Something isn't working label Dec 21, 2023
@PhilippSalvisberg PhilippSalvisberg modified the milestones: v5.0, v4.5 Dec 22, 2023
@PhilippSalvisberg
Copy link
Collaborator Author

PhilippSalvisberg commented Jan 11, 2024

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 PhilippSalvisberg changed the title False positive for G-6020 when dynamic SQL is not a INSERT, UPDATE, DELETE or MERGE statement False positive for G-6020 when dynamic SQL is not a INSERT, UPDATE or DELETE statement Jan 11, 2024
@PhilippSalvisberg
Copy link
Collaborator Author

fixed with Azure DevOps commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant