Tom Kyte posted a blog entry that included a test illustrating why not using bind variables can make your code susceptible to SQL Injection. I’ve heard Tom and others talk about this subject before, but to be honest I just accepted their premise and never spent much time experimenting with it. So after reading the post and Googling for SQL Injection, I found a nice example by David Litchfield and came up with this as a way to attack the code included in the test.
create or replace procedure inj( p_date in date )
as
l_rec all_users%rowtype;
c sys_refcursor;
l_query long;
begin
l_query := '
select *
from all_users
where created = ''' ||p_date ||'''';
dbms_output.put_line( l_query );
open c for l_query;
for i in 1 .. 5
loop
fetch c into l_rec;
exit when c%notfound;
dbms_output.put_line( l_rec.username || '.....' );
end loop;
close c;
end;
/
SQL> ALTER SESSION SET NLS_DATE_FORMAT = '" '' or ''a'' = ''a"';
Session altered.
SQL> exec inj(''' or ''a'' = ''a');
select *
from all_users
where created = ' ' or 'a' = 'a'
ILO.....
ILMDS.....
BI.....
PM.....
SH.....
PL/SQL procedure successfully completed.
SQL>
Tom subsequently posted a followup entry and my example wasn’t too far off and did illustrate the point.
I thought question 3: “how to best protect against that attack” was also interesting. Here’s my attempt at creating a “safer” procedure:
create or replace procedure inj1( p_date in date )
as
l_ctr pls_integer := 1;
begin
for l_rec in
( select * from all_users
where created = p_date )
loop
if l_ctr <= 5 then
dbms_output.put_line( l_rec.username || '.....' );
else
exit;
end if;
--
l_ctr := l_ctr + 1;
end loop;
end;
/
The big difference I think is that there was no reason to use dynamic SQL in the original example. Now I understand it was for illustration, but realistically in that case dynamic SQL just added to the ease of attacking the code. I still use a bind variable but I’ve eliminated the dynamic SQL from the code. The results?
SQL> ALTER SESSION SET NLS_DATE_FORMAT = '" '' or ''a'' = ''a"';
Session altered.
SQL> set serveroutput on
SQL> exec inj1(''' or ''a'' = ''a');
PL/SQL procedure successfully completed.
SQL> select username,
to_char(created,'MM/DD/YYYY HH24:MI:SS')
from all_users where trunc(created) =
to_date('10/14/2007','MM/DD/YYYY');
USERNAME TO_CHAR(CREATED,'MM
------------------------------ -------------------
SYS 10/14/2007 15:05:30
SYSTEM 10/14/2007 15:05:31
OUTLN 10/14/2007 15:05:32
DIP 10/14/2007 15:07:32
XDB 10/14/2007 15:31:47
DBSNMP 10/14/2007 15:20:35
WMSYS 10/14/2007 15:23:01
EXFSYS 10/14/2007 15:30:19
CTXSYS 10/14/2007 15:30:45
ANONYMOUS 10/14/2007 15:31:47
ORDSYS 10/14/2007 15:36:15
ORDPLUGINS 10/14/2007 15:36:16
SI_INFORMTN_SCHEMA 10/14/2007 15:36:16
MDSYS 10/14/2007 15:36:16
OLAPSYS 10/14/2007 15:46:49
MDDATA 10/14/2007 15:47:54
SYSMAN 10/14/2007 15:54:03
MGMT_VIEW 10/14/2007 15:58:29
FLOWS_FILES 10/14/2007 15:59:24
APEX_PUBLIC_USER 10/14/2007 15:59:24
FLOWS_030000 10/14/2007 15:59:24
OWBSYS 10/14/2007 16:10:50
SCOTT 10/14/2007 16:13:15
23 rows selected.
SQL> exec inj(to_date('10/14/2007','MM/DD/YYYY'));
select *
from all_users
where created = '10/14/2007'
PL/SQL procedure successfully completed.
SQL> exec inj1(to_date('10/14/2007 16:13:15','MM/DD/YYYY HH24:MI:SS'));
SCOTT.....
PL/SQL procedure successfully completed.
SQL> exec inj(to_date('10/14/2007 16:13:15','MM/DD/YYYY HH24:MI:SS'));
select *
from all_users
where created = '10/14/2007'
PL/SQL procedure successfully completed.
SQL>
The attack didn’t work, but there are other problems. Because of the date usage the query isn’t really practical as you can see in the example. Unless you format or trunc the “created” column it’s not very easy to make this procedure, or the original for that matter, work as intended.
My point though is not to take issue with a simple example, but to raise the issue of input validation. In playing around with this I was reminded of how hard it can be to really code the checks needed to insure that input is properly validated. Most of the time we don’t and we just throw an exception when bad input causes a failure at run time. This practice though, can also lead to SQL Injection if you’re not careful. Even Tom and several commenters to his thread point out, that another way to protect the code is to validate the input. Whether by using format masks as Tom mentions, which is probably the easiest way, or by actually decoding and testing the input. The problem of course is that actually validating input can be hard and leads to more coding and possibly errors. Of course getting your database hacked isn’t that great of an alternative either.
Entries