oracleplsqlsql-injectioncheckmarx

Avoid SQL injection in incoming query coming in in-parameter


I have a procedure which will receive query in in parameter. I'm running the incoming query inside a procedure, and the Checkmarx tool detecting SQL injection in my_cursor.

How do I solve this?

I tried to introduce dbms_assert.noop function but no use

`create procedure test(common_query in varchar2)
as
sql_qry varchar2(2000);
Type cursor_type is ref cursor;
my_cursor cursor_type;
begin
sql_qry:=  common_query;
open my_cursor for sql_qry;
fetch my_cursor  into :name;
close my_cursor ;
end;`

I tried to introduce dbms_assert.noop function but not resolving


Solution

  • SQL injection is what you are doing. You can either embrace it or re-architect your code, but don't pretend it's not happening.

    Embrace the SQL injection (carefully)

    If you are writing a generic SQL execution tool, like an imitation Oracle SQL Developer or a website like dbfiddle.uk, then you need to admit to yourself that you're doing SQL injection. If this is the case, then your problems cannot be solved by a simple DBMS_ASSERT or "sanitizing your database inputs" like the famous XKCD comic implies.

    You will need to thoroughly isolate and protect the database user through careful privilege review, DDOS protection (e.g. use resource manager to avoid regular expression attacks), and probably other things I cannot think of. Add some "DANGER" comments to the code, so future developers understand the risk and will be on the lookout for security bugs. Have the code peer reviewed multiple times.

    Before you go down this path, don't bother trying something as simple as "only allow statements that start with SELECT". There are many sneaky ways to make a SELECT statement change and alter your database. And even reading the wrong thing can be dangerous, so you need to consider privileges anyway.

    Or, at the very least, if you swear up and down that you can trust all of the people with access to an internal-only application, carefully document everything and put up large warning signs to ensure nobody ever exposes your user interface externally.

    Finally, you can change CheckMarx to exclude this inevitably non-compliant code. Almost every security rule has an exception.

    Re-architect (the most likely solution)

    As Bill Karwin suggested, virtually all programs use pre-approved SQL statements, where the unknown values are plugged in as bind variables. In the less likely, but not super rare, case that you need to make a table name dynamic, then you can use DBMS_SQL to check that one simple input. Or, as Paul W suggested, do you even need a stored procedure at all? (But don't just push your SQL injection into your frontend.) What kind of processing and common logic are added by your stored procedure? And is it something that can be handled through a database feature like a view or virtual private database?