0

Given this MySQL stored procedure:

CREATE PROCEDURE customer.`getCustomers5`(
sdf varchar(1000)
)
BEGIN

set @se  = concat('select * from customer.customertbl where id=', sdf);


PREPARE stm1 from @se;

EXECUTE  stm1;

END;

Is it possible to do SQL injection into this store procedure even if the front end that called this stored procedure uses PDO parameter/data binding?

I need to build a query dynamically (dynamic where clause) before calling it.

if it's possible to do SQL injection, is there any method to counter this problem?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Melvin
  • 377
  • 2
  • 7
  • 19
  • http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php. in short with PDOs you should be safe. aaand http://php.net/manual/en/pdo.prepared-statements.php – Sergey Benner Feb 11 '12 at 10:55
  • @SergeyBenner it seems you don't get the question. – Your Common Sense Feb 11 '12 at 12:03
  • -1 for asking a vague question that turns out "no, I didnt mean that, the code is totally different". – Your Common Sense Feb 11 '12 at 13:24
  • In your example code, if you do this instead `CREATE PROCEDURE customer.`getCustomers5`(sdf varchar(1000)) BEGIN select * from customer.customertbl where id=sdf; END;` No SQL Injection would be possible. But yes, dynamic queries need to be parameterized, else SP alone wouldn't provide any protection from sql injection. – Allen King Nov 12 '20 at 19:28

2 Answers2

2

You are just using prepared statements wrong.
You have to bind parameters, not concatenate them.

DELIMITER // 
CREATE PROCEDURE customer.`getCustomers5`(sdf varchar(1000)) 
BEGIN 
  PREPARE stm1 from 'select * from customer.customertbl where id=?'; 
  SET @a = sdf;
  EXECUTE stm1 using @a; 
END//
DELIMITER ;
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • the SQL statement which I want to do is dynamic, the number of parameters is not certain. in this case the set @a method wont work. I guess i still need to built the SQL @ Application level . – Melvin Feb 11 '12 at 13:20
  • Sorry, buddy but your comment looks nonsense. set @a method does use the stm variable which is apparently not dynamic statement at all. If you need to create SQL dynamically - no problem. Create a prepared SQL with placeholders, not values. It's not a big deal. Only thing you need is to learn and to listen. – Your Common Sense Feb 12 '12 at 06:56
-1

If your parameter is varchar and you send a string, then yes, it is possible because even if you use PDO, it would still be a ANY string.

You should define sdf as your id type (is it integer? if not, make it integer!) and then the PDO parameter will be escaped and avoid SQL Injection.

A good practice is to avoid creating dynamic queries in a Stored Procedure and build the query in your application.

matheuzzy
  • 472
  • 3
  • 10
Stelian Matei
  • 11,553
  • 2
  • 25
  • 29
  • Yes, Id is integer, i was just testing the limit. another question, if there are many sql statements involved, would it be better to consolidate in a stored procedure or application level ? – Melvin Feb 11 '12 at 11:00
  • a stored procedure is OK. but you will have to watch out for all of your locks/lock waits and so on. – Sergey Benner Feb 11 '12 at 11:06
  • If you are perfoming many SQL statements in order to compute a single result, it would be best to consolidate in a stored procedure. That way the application doesn't need to make several queries and keep data locally for further processing. So, try to create queries as complex as possible to make all the computations on the database server and you can put them in stored procedures for easier access/maintenance. You should still avoid creating dynamic queries in stored procedures, maybe add more parameters or create more procedure to match each scenario. – Stelian Matei Feb 11 '12 at 11:08
  • OK, i guess I will create all my dynamic query @ application level and bind them using parameter binding.. other than that, it will go to SP . – Melvin Feb 11 '12 at 11:15
  • what does it mean, "PDO parameter will be escaped"? And what if the id were of string type? – Your Common Sense Feb 11 '12 at 11:47