Am I safe against SQL injection
-
06-07-2019 - |
Question
I would like to know if I'm safe against SQL injection when I use something like that with PostgresSQL:
CREATE or REPLACE FUNCTION sp_list_name( VARCHAR )
RETURNS SETOF v_player AS '
DECLARE
v_start_name ALIAS FOR $1;
r_player v_player%ROWTYPE;
v_temp VARCHAR;
BEGIN
v_temp := v_start_name || ''%'';
FOR r_player IN
SELECT first_name, last_name FROM v_player WHERE last_name like v_temp
LOOP
RETURN NEXT r_player;
END LOOP;
RETURN;
END;
' LANGUAGE 'plpgsql' VOLATILE;
I want to use this function to list player's name beginning with a letter.
select * from sp_list_name( 'A' );
gives me players with last name beginning with A.
I tried to inject sql with
select * from sp_list_name( 'A; delete from t_player;--' );
select * from sp_list_name( '''; delete from t_player;--' );
Am I safe ?
Which case I could be injected ?
Regards
Solution
In terms of your procedure you seem safe as the variable in the SP won't be expanded into code, but you can still expose yourself if you don't use a parameterized query like "SELECT * FROM sp_list_name(?);
" in your appplication code. Something like "SELECT * FROM sp_list_name('$start_name');
" could be subverted by a user passing a start name of "');delete from t_player where last_name NOT IN ('
". So use a parameterized query or sanity check your inputs in your program.
NB: To others, please note that a variable in a stored procedure will not expand into code even if it contains a ' or ;, (excluding passing it to EXECUTE, for which you would use quote_literal
, not hand-rolled replace
functions) so replacing ; or ' is totally unnecessary (in the stored procedure, the application using it is a different story, of course) and would prevent you from always finding the "tl;dr
" or "O'Grady
" teams.
Leo Moore, Karl, LFSR Consulting: v_temp_name
in the stored procedure will NOT be expanded into code in the SP (no EXECUTE), the check would need to be done n the application, not the SP (or the OP could just use a parameterized query in their app code, instead). What others are suggesting is similar to worrying about
my $bar = "foo; unlink('/etc/password');";
my $baz = $bar;
actually running the unlink in the absence of an eval.
OTHER TIPS
Rule #1 to prevent against sql injection: Sanitize all input that is coming from someone/something you cannot trust/have no control over.
The problem itself does not lie within the database code, but from the application that is executing those statements.
The proper way to protect against SQL Injection is via White Listing* - the long and short is set the characters that you're going to accept and filter them out.
The incorrect way is to Black List - Black listing what characters aren't accepted is going to lead to trouble, because you can't keep up with attackers. There are ways around black lists via ASCII tables, escape characters and what not.
Also, here's a nice cheat sheet to try out on your site. Run some tests and try to get things to fail.
*
In the application, not the DB (Thanks James)
You are not generating SQL for yourself, so this looks safe (to me).
I do not know where the data you are calling your stored procedure is coming from. So you still have to guard against buffer overflows etc.
Ref. White Listing. This is OK if certain other conditions are followed. It is absolutely imperative to break all input down into it's simplest form so don't just check for SQL query names, single quotes etc. They can be represented or encoded using other character sets and it's this that forms part of the white list not specifically SQL key words themselves.
I was working for a particular client that allowed username /password to a protected resource (which ultimately could get you an access pass into secure parts of an Airport!). You could circumvent the logon field by entering ' and then building SQL queries from there to retrieve user accounts and passwords.
The problem was the client had already blown £200k building the site with a vendor that appeared to have never done web development before. The fix was a further £60k which was a validate func() that was only checking for union, select, having et al key words. When asked about what they did for canicolisation /encoding (which I had to then explain) it was tumbleweed time.
The Dev house and the (expensive) Project got canned.
You could consider validating the content of
v_start_name. Check the string for semi colons, comment chars, equals, etc. Remember to check for both the Chars and the Hex values. Remember to allow for a hyphenated name, e.g. 'Smith-Brown' is likely acceptable 'Smith--Brown' is potentially injection.
If not familiar with Hex in SQL Injection the following are quick intros
http://www.arejae.com/blog/sql-injection-attack-using-t-sql-and-hexadecimal.html
http://www.securityfocus.com/infocus/1768
DECLARE v_start_name ALIAS FOR $1; r_player v_player%ROWTYPE; v_temp VARCHAR; BEGIN -- new pseudo code here if v_start_name has bad chars exit with error message -- end pseudo code here v_temp := v_start_name || ''%''; FOR r_player IN SELECT first_name, last_name FROM v_player WHERE last_name like v_temp LOOP RETURN NEXT r_player; END LOOP; RETURN; END;
Regards Karl
Just do a replace on your v_start_name to get rid of ";" etc. ie
v_clean_name VARCHAR;
Select v_clean_name = Replace(v_start_name,';','');
This will replace the ; with blanks foiling a SQL injection attack
For more details see String Functions in PostgresSQL
As LFSR Consulting has also commented. It is better to WhiteList (ie Do not process any input with invalid characters such a ';') rather than BlackList (ie try to clean the data as the user could do a SQL injection attack on your Replace too).
For more info have a look at SQL Injection Attacks