How is this MySQL query vulnerable to SQL injection?

Question

In a comment on a previous question, someone said that the following sql statement opens to me before SQL injection:

select ss.*, se.name as engine, ss.last_run_at + interval ss.refresh_frequency day as next_run_at, se.logo_name from searches ss join search_engines se on ss.engine_id = se.id where ss.user_id='.$user_id.' group by ss.id order by ss.project_id, ss.domain, ss.keywords 

Assuming the $userid variable is properly escaped, how does that make me vulnerable, and what can I do to fix it?

Answer 1

Assuming it is properly shielded, it does not make you vulnerable. The fact is that escaping properly is more difficult than at first glance, and you condemn yourself to run away properly every time you make such a request. If possible, avoid all these problems and use prepared statements (or related parameters or parameterized queries). The idea is to allow the data access library to exit the values ​​correctly.

For example, in PHP using mysqli :

 $db_connection = new mysqli("localhost", "user", "pass", "db"); $statement = $db_connection->prepare("SELECT thing FROM stuff WHERE id = ?"); $statement->bind_param("i", $user_id); //$user_id is an integer which goes //in place of ? $statement->execute(); 

Answer 2

Each SQL interface library used has some support for binding parameters. Do not try to be smart, just use it.

You can really think / hope that you managed to avoid unnecessary things, but it is not worth the time when you do not.

In addition, several databases support prepared instruction caching, so proper execution can also bring you profitability gains.

Lighter, safer, faster.

Answer 3

If it is properly shielded and checked, then you have no problem.

A problem occurs when it is not executed properly or not verified. This may occur by careless coding or surveillance.

The problem is not in specific cases, but in the template. This pattern makes SQL injection possible, while another pattern makes it impossible.

Answer 4

If $ user_id is hidden, then you should not be vulnerable to SQL Injection.

In this case, I also guarantee that $ user_id is numeric or integer (depending on the exact type required). You should always limit the data to the most restrictive type you can.

Answer 5

I think “Escaped Right” is the key word. In your last question , I make the assumption that your code is copied from your production code, and since you asked about joining the three tables, I also make the assumption that you did not perform the correct escaping, so my observation about SQL Injection attack.

To answer your question, as so many people described here, IF , this variable has been "properly escaped," then you have no problem. But why bother with this? As some people have pointed out, sometimes right deliverance is not a simple matter. There are templates and libraries in PHP that make SQL injection impossible, why don't we just use it? (I also purposely make the assumption that your code is actually PHP). Vinko Vrsalovic answer can give you ideas on how to approach this issue.

Answer 6

This operator as such is not really a problem, its “safe”, however I don’t know how you do it (one level in the API stack). If $ user_id is inserted into the statement using string operations (for example, if you allow Php to automatically populate the statement), then this is dangerous.

If it is populated using the binding API, then you are ready to go.

Answer 7

All answers are good and correct, but I feel that I need to add that the prepare / execute paradigm is not the only solution. You should have a database abstraction level rather than using library functions directly, and this level is a good place to explicitly exclude string parameters, whether you allow prepare to do it yourself or do it yourself.