2

I have a Stored Procedure which gets 6 input queries and the inputs are the same as where clause of a query. For instance where x=1 and z=5 and e=4. The Application makes these input strings and pass it onto my procedure.This is a part of my procedure in which I'm checking the inputs in order to validate the content of each input query so that they do not contain keywords like "select", "update", "delete" or equivalents.I want to know whether there are better ways to do that.

create or replace procedure app_error_test(query1 nvarchar2,
                                           query2 nvarchar2,
                                           query3 nvarchar2,
                                           query4 nvarchar2,
                                           query5 nvarchar2,
                                           query6 nvarchar2) is
queryconcat nvarchar2(30000);
begin

--**************** validate content of input queries

queryconcat := lower(nvl(query1, '') || nvl(query2, '') ||
                     nvl(query3, '') || nvl(query4, '') ||
                     nvl(query5, '') || nvl(query6, ''));

 if (queryconcat like '%drop%' or 
      queryconcat like '%delete%' or
       queryconcat like '%execute%' or 
        queryconcat like '%truncate%' or
         queryconcat like '%create%'  or 
          queryconcat like '%update%'  or
           queryconcat like '%insert%') then

     RAISE_APPLICATION_ERROR(-20032, 'ILLEGAL CONTENT');
  end if;
end;

Thanks in advance

1 Answer 1

1

I would use regex_like:

select column_value
     , case
           when regexp_like(' '||column_value||' ','\s(drop|execute|truncate|create|insert|update|delete|merge)\s', 'i')
               then 'Yes'
       end as illegal
from   table(ora_mining_varchar2_nt
             ('drop table customers', 'just dropping by', 'undroppable'));
COLUMN_VALUE              ILLEGAL
------------------------- -------
drop table customers      Yes
just dropping by          
undroppable               

I've added merge to the list as I assume you are trying to prevent SQL injection. If so, there are still risks even if you filter for keywords. execute isn't a SQL keyword, though. Did you mean call?

Your procedure would become:

create or replace procedure app_error_test
    ( query1 nvarchar2
    , query2 nvarchar2
    , query3 nvarchar2
    , query4 nvarchar2
    , query5 nvarchar2
    , query6 nvarchar2 )
as
    queryconcat nvarchar2(30000) :=
        query1||' '||query2||' '||query3||' '||query4||' '||query5||' '||query6;
begin
    if regexp_like(' ' || queryconcat || ' ', '\s(drop|call|truncate|create|insert|update|delete|merge)\s', 'i')
    then
        raise_application_error(-20032, 'Illegal content in supplied SQL: '||queryconcat);
    end if;
end;
6
  • Thanks for your answer . I'm trying to understand in . I was wondering if you could tell me more about others risks . You said there are still risks and yes by execute , I mean call.
    – Pantea
    Jul 19, 2020 at 13:42
  • Could you please explain your answer a little bit more? It seems like a very good answer but it's a bit complicated for me to understand.Where are the 6 input parameters ?
    – Pantea
    Jul 19, 2020 at 13:45
  • 1
    By other risks, I was thinking of the scenario where a select query is unexpectedly extended to show more than it should. blogs.oracle.com/sql/what-is-sql-injection-and-how-to-stop-it Jul 19, 2020 at 13:45
  • 1
    The table(ora_mining_varchar2_nt()) part of my query is just to generate some data without creating a table, and the case expression is just to make it clear which values match the pattern and which don't. The regexp_like() expression is the main bit. Jul 19, 2020 at 13:49
  • 1
    Yes, exactly. I've added it to my answer to make it clearer. Jul 19, 2020 at 13:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.