Is this MySQL stored procedure still at risk of SQL injection if it uses prepared statements?

Question

I'm fairly new to MySQL and I've searched around as much as I can, but I was hoping someone could provide me with a definitive answer.

I've been looking at issues with a stored procedure in my MySQL database. It takes a single string parameter from an external source as a search value and returns results based on that. However, whenever a ' (single quote/apostrophe) is typed, it returns an error. Looking into it, I was advised that the SP as it is is vulnerable to SQL injection and that by resolving that the issues with the single quote will be resolved. This is the stored procedure:

BEGIN
SET @t1=CONCAT("SELECT `ID` as `Id`, `NAME` as `Name` FROM SEARCH_TABLE WHERE `NAME` like","'",searchString,"%'"," LIMIT 20");
    PREPARE stmt3 FROM @t1;
    EXECUTE stmt3;
    DEALLOCATE PREPARE stmt3;
END

Everything I've looked at says that the best way to avoid SQL injection is to use prepared statements, so is it still at risk? Additionally, to resolve the issue with the single quote, I've seen recommendations to add another single quote wherever one is typed (so ' will become ''). Is this the best way to fix the error?

Thanks for the help

already answered[How does a PreparedStatement avoid or prevent SQL injection?](https://stackoverflow.com/questions/1582161/how-does-a-preparedstatement-avoid-or-prevent-sql-injection) — Divyesh patel, Oct 11 '19 at 08:30
"Use prepared statements" is only part of the answer for preventing SQL injection. "Use query parameters instead of adding untrusted content with string-concatenation" is the more accurate answer. But using query parameters requires using a prepared statement. — Bill Karwin, Oct 11 '19 at 19:03

score 2 · Answer 1 · edited Oct 11 '19 at 20:59

2

While you are using a prepared statement, you aren't using positional placeholders ? for the variable part of your statement. Therefore, your select query, as you wrote it, is still prone to SQL injection. Here is an updated version:

BEGIN
SET @t1 = CONCAT('SELECT ID AS Id, NAME AS Name, FROM SEARCH_TABLE WHERE NAME LIKE ? LIMIT 20');
PREPARE stmt1 FROM @t1;
SET @a = CONCAT('%', searchString, '%');  -- assuming searchString is defined somewhere
EXECUTE stmt1 USING @a;

We bind %?% using the CONCAT() function, and notice that the LIKE placeholder is just a single ?. Also, your query should ideally have an ORDER BY clause assuming it is using LIMIT.

edited Oct 11 '19 at 20:59

Salman A

262,204
82
430
521

answered Oct 11 '19 at 08:33

Tim Biegeleisen

502,043
27
286
360

Thanks for the response. I'm attempting the SQL you provided above but I'm getting a syntax error near `PREPARE stmt1 FROM @t1; SET @a = CONCAT("%", searchString, "%"); EXECUTE stmt1` Any idea what could be causing it? – optional_gun Oct 11 '19 at 11:18

score 1 · Answer 2 · answered Oct 11 '19 at 08:50

Yes, your code is open to SQL injection because the prepared statement has the user supplied input concatenated as-is. Assuming searchString contains the string ' OR '%' = ' then this:

SET @t1=CONCAT("SELECT `ID` as `Id`, `NAME` as `Name` FROM SEARCH_TABLE WHERE `NAME` like","'",searchString,"%'"," LIMIT 20");

Would result in:

SELECT `ID` as `Id`, `NAME` as `Name` FROM SEARCH_TABLE WHERE `NAME` like'' OR '%' = '%' LIMIT 20

It'll allow someone to pass arbitrary where clause. Also, a single ' in the user input will break the query.

Is this MySQL stored procedure still at risk of SQL injection if it uses prepared statements?

2 Answers2