1

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?

Dharman
  • 30,962
  • 25
  • 85
  • 135
Andy Bbop
  • 21
  • 2
  • 6
    Why does it matter? SQL injection isn't the only problem that using prepared statements solves. Don't waste time looking for excuses not to use them. – Barmar Jan 06 '23 at 16:44
  • 2
    Because blindly following a paradigm without understanding it properly is not the path to wisdom. – Andy Bbop Jan 06 '23 at 16:46
  • Sure, but SQL injection and prepared statements are not obscure things. There are mountains of relevant questions and answers on this site, and articles elsewhere on the internet that explain the respective risks and benefits. This question is simply asking for someone to re-explain all that specifically to you in the context of your contrived example case. – Sammitch Jan 06 '23 at 19:49
  • No, I think you will find that you all wanted to re-explain parameterised queries despite them not actually being part of the question. I simply wanted to know if there were risks in using some post data as a comparison operator without it ever entering a database query, and if there is such an abundant range of answers to such a simple question, why cant I find them? – Andy Bbop Jan 06 '23 at 21:05

2 Answers2

3

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 $_SESSION because 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 input

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • Could you elaborate on how? – Andy Bbop Jan 06 '23 at 16:49
  • I think you need to explain the risk, not just give a blanket "yes". – Barmar Jan 06 '23 at 16:49
  • Every case is an obscure case until it happens..... then no matter how obscure it is, it's a massive pain in the fish-sack! – Martin Jan 06 '23 at 17:01
  • This is all well and good and useful to learn, but the question was not whether session variables can be manipulated, or whether parameterised queries can prevent other types of attack. The real question was whether it's possible to be at risk just by making a comparison to a value picked up by post. This is also not a duplicate question, for the same reason. – Andy Bbop Jan 06 '23 at 17:19
  • Hi, thanks for the additional response on that, my original question was perhaps open to too many other tangents, but that is very useful thanks. Yes just being sloppy with $POST. – Andy Bbop Jan 06 '23 at 17:56
1

This answer is an additional to Bill Karwin's answer which I agree with. However, you made an interesting point, nay, assumption here:

No one can mess with the session variable so I am fine

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"] ...

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.

Martin
  • 22,212
  • 11
  • 70
  • 132
  • "or so I gather." I perhaps forgot to mention that session variable is generated in a parameterised query, so will only be generated in a legitimate login attempt, it is created from data bound in that query and is based on the unique auto indexing integer ID number created by the DB for that user, so given those points, unless I have already left a giant barn door open anyway, the session variable should be pretty secure from being manipulated, is this not the case? – Andy Bbop Jan 06 '23 at 20:18
  • @AndyBbop how the variable is generated isn't a big deal, the way it is stored and protected on the system *is*. Security is onion rings, you say things like "pretty secure" which screams that it's not "totally secure" because *nothing* is *totally* secure, so therefore you need to factor in the dangers (however small) of variables into your SQL code and use Parameterised queries to add an extra layer to the onion of protection for your account and system. – Martin Jan 06 '23 at 22:08
  • @AndyBbop so take away from this Q&A : use Parameterised queries. Don't use `$_SERVER['PHP_SELF']`. Always add skins to your security onion. Explore PHP best practise for your code and protect yourself as much as possible. `:-)` – Martin Jan 06 '23 at 22:10