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

dynamic-sqloracle-11g-r2plsqlSecuritysql-injection

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

Best Answer

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;
Related Question