How to prevent a highly dynamic PL-SQL query from injection using bind variables

Question

I have a procedure as you can see below:

create or replace procedure Injection_test(qry1 varchar2,
 qry2 varchar2,
 user_id varchar2) 
is
 final_query varchar2(1000) := '';
begin
 if (qry1 is not null) then
 final_query := 'select c_num from z_test_a where userid = ''' ||user_id || ''' and ' || qry1;
 if (qry2 is not null) then
 final_query := final_query || 'intersect ';
 end if;
 end if;
 if (qry2 is not null) then
 final_query := final_query || ' select c_num from z_test_b where userid = ''' ||user_id || ''' and ' || qry2;
 end if;
--dbms_output.put_line(final_query);
 execute immediate 'insert into Z_final_result (c_num)'||final_query;
 commit;
end;

As you can see , the variable final_query is highly dynamic based on two input variablesqry1 and qry2. Since I'm new to Oracle pl-sql injection , I don't exactly know how to prevent this piece of code from sql injection . I know the basic concepts of injection and how it occurs -for example when we have null or 1=1 for one of the input variables,all the records of the table will be inserted in the final table- but I do not know exactly how to change this procedure and usebind variables to prevent injection from happening.

I was wondering if you could help me out with this Thanks in advance

• Pantea: Nobody has a good answer?

Answer 1

I'm not sure there is a good answer. In general, string concatenation to build your SQL is always a bad idea and should be avoided whenever possible. That said, it would be difficult to avoid in your case except for replacing user_id references with bind variables. Other than that, you will have to come up with some custom logic to validate the content of qry1 and qry2 to ensure they only contain what you expect them to contain. Perhaps a length limitation, or a check of keywords or column names? Either that, or replace qry1 and qry2 with explicit values for the columns in each table, including wildcards, then turn those into bind variables.

As long as you have to accept actual SQL code snippets as your input, I would recommend something like this:

create or replace procedure Injection_test(p_qry1 varchar2,
 p_qry2 varchar2,
 p_user_id varchar2) 
is
 final_query varchar2(1000) := '';
begin
 -- put some logic here to validate the content of p_qry1 and p_qry2 to make sure
 -- they don't contain keywords like "select", "update", "delete", "1=1", ";" or
 -- equivalents, and/or that they do contain things you expect to see. Raise an
 -- application error anything unexpected is found
 if p_qry1 is not null
 then
 final_query := 'select c_num from z_test_a where userid = :user_id and ' 
 || qry1;
 if p_qry2 is not null then
 final_query := final_query || ' intersect ';
 end if;
 end if;
 
 if p_qry2 is not null then
 final_query := final_query 
 || 'select c_num from z_test_b where userid = :user_id and ' 
 || qry2;
 end if;
 if final_query is not null
 then
 final_query := 'insert into Z_final_result (c_num) ' || final_query;
 --dbms_output.put_line(final_query);
 if instr(final_query,'intersect') > 1
 then 
 execute immediate final_query using p_user_id, p_user_id;
 else
 execute immediate final_query using p_user_id;
 end if;
 commit;
 end if;
end;