If I have a user input form, but the data from this is used purely to make comparisons in PHP, can it be subject to a SQL injection risk? I assume not, and that it is only where I want to pick up such data via POST or GET and then put it into a database query that it becomes a risk.
For example.
Lets say I return a dataset using a session variable of user_id created at login.
$sql="SELECT * FROM leads where user_id='".$_SESSION[user_id]."'";
No one can mess with the session variable so I am fine to just use this and a bog standard mysqli_query to return my result set, or so I gather.
I now have a small form with months of the year on a select box, and use echo htmlspecialchars($_SERVER["PHP_SELF"]); to refresh the page on form submission and filter the results of the report which are printed to the HTML page.
if($results = mysqli_query($link, $sql)){
while($row = mysqli_fetch_array($results )) {
if($row["time_stamp"]>$POST["select_box") { Do some other stuff...}
Is there no risk of SQL injection because I am not actually passing the select box value into a database query?
Or Does the mere fact that I am picking up data from a user input, via POST, and performing some kind of action with it expose me to risk?
2
Answers
Yes, this is an SQL injection risk.
You may assume that
$_SESSION[user_id]is set by your login system. How confident are you about this? Is there no way an attacker could pollute your session data?Is the user id guaranteed to be an integer? Or can your user id’s be strings? If so, can they have content that would influence the syntax of an SQL query? Like could it contain a quote character?
There’s also a hidden bug in your example.
$_SESSION[user_id]uses a PHP constant as the key, and it isn’t necessarily the same as$_SESSION['user_id']if the constant has been defined to some string other than'user_id'. Undefined constants in PHP default to a string value the same as the name of that constant, but if somehow the constant becomes defined to some other string, it could reference a different session variable.These are admittedly obscure cases. Perhaps the risk is minimal. But why eschew the known solution to SQL injection, which is to use query parameters?
Besides that, you’re almost always better off using query parameters instead of concatenating strings and variables. It leads to code that is easier to read and debug, and it’s actually better for performance.
Re your comment:
You show comparing the result of the query to your POST data. That’s after the query has already executed.
Your POST data cannot affect your SQL query, because you don’t interpolate POST data into the SQL query.
I was focused on your use of
$_SESSIONbecause that’s the only variable that could be involved in SQL injection in your example.P.S.: Not related to your question, but I think you should use
$_POST, not$POST, in your code if you want to reference the superglobal for POST inputThis answer is an additional to Bill Karwin’s answer which I agree with. However, you made an interesting point, nay, assumption here:
Oh? How come? This depends heavily that the rest of your PHP code is safe, your production environment is properly secured, dangerous scripts are avoided and system-editing native PHP functions are locked down.
You’re server system / PHP system / Session system is probably at risk from a number of other factors such as cookie manipulation, CSRF, XSS, or even file system breach on your website account.
While these issues are massive in themselves, and many of them can lead to MySQL compromise anyway; don’t assume just because one door appears locked that all doors are locked.
Your Edit :
Server PHP_SELF can be compromised and should not be used.
The fact you’re making these assumptions above shows there’s a non-zero risk to your system and so you should be using the safest SQL interaction methods you can.