1

I'm sorry about the title being a little unclear but I'm new where. I was wandering around StackOverflow and came across an answer stating that when executing a SQL query, data should never be fed directly ($db->query("SELECT * FROM users WHERE id LIKE $id")), but should be bound in a prepared statement instead ($db->prepare("SELECT * FROM users WHERE id LIKE ?)->execute(array($id))).

Now, I'm aware of SQL Injection and that I should use a code that looks like the latter, but my question is; is that always the case? Like, if I had the following code:

$db->query("SELECT * FROM products WHERE id LIKE $id")

Let's suppose that I gave that $id from within my code, and that it is not an input from the user, would I still have to use a prepared statement? Or would I be fine with the first example?

TAccount
  • 80
  • 6
  • Always parameterize. A developer later could redefine the variable to use user input. You also can get second level injections depending on where source comes from. – user3783243 Mar 03 '20 at 05:12
  • You're right, I didn't think about that! Thanks for your answer, I will wait for other answers, maybe there's even more to it – TAccount Mar 03 '20 at 05:25
  • 1
    Striving for inconsistency is no valid reason to tiptoe around parameterization. Also: "2nd order injection". – mario Mar 03 '20 at 05:59
  • 1
    What's the potential upside that you see to not parameterizing some queries? Saving a few seconds of typing? All rules are made to be broken, but only when there's a very good reason to do so, and saving a few seconds is not a very good reason. – Greg Schmidt Mar 03 '20 at 06:09
  • Yeah, that's true too – TAccount Mar 03 '20 at 06:21
  • Also, someone just closed this question as a duplicate and linked to a different question; ok PHP mods – TAccount Mar 03 '20 at 06:24
  • I voted to reopen because this is in fact a different question. I will just point out that SQL injection is not always about defeating malicious hackers — it's often to protect well-intentioned, legitimate content that happens to contain special characters (like apostrophes). – Bill Karwin Mar 03 '20 at 07:20
  • I agree with Bill and the others. When you use parameters, it is clear to everyone (and, to the SQL engine) what is "part of the query" and what is "input to a particular run of that query." Furthermore, the value of the parameter is passed without encoding it as a string. It's clean, it's just as easy to do as stringing, and unlike stringing it always works. Good habit to make, and keep. – Mike Robinson Mar 04 '20 at 20:46

1 Answers1

0

This is safe because your code sets $id to a literal, constant value. There is no way an untrusted value can be used.

$id = 123;
$db->query("SELECT * FROM products WHERE id LIKE $id");

This is safe because doing a type-casting removes any possibility of special characters that could cause a problem with respect to SQL injection. A plain integer is safe.

$id = (int) $_GET['id'];
$db->query("SELECT * FROM products WHERE id LIKE $id");

But the fact remains that once you use different methods of writing queries, sometimes using variable expansion in strings, and sometimes using bound query parameters, you make your code harder to maintain.

Consider the lifetime of this code. How confident are you that the next junior programmer who takes over support for this code will understand the risks of SQL injection well enough to judge when it's safe to use variable expansion, and when they should use bound query parameters?

It's safer to establish a coding standard that you always use bound query parameters when you want to combine a variable with an SQL query, because this coding standard is easier to document and easier to enforce. Therefore it's less likely to allow unsafe cases by accident.

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • Bill, I'd just tell anyone: *"Use parameters. All the time. Just because it's the right thing to do."* It's really no more difficult than constructing a string, it's safer, it's more efficient and it always works. So, the Nike rule applies (IMHO): "Just Do It." – Mike Robinson Mar 04 '20 at 20:47
  • I agree that it's the right thing to do, but the reason "just do it" is not convincing to some developers. You have to give them more of a practical reason. – Bill Karwin Mar 05 '20 at 03:00