37

The poor PHP tag is still suffering from a lot of questions caused by the most basic syntax issues.

One of such questions is constructing an SQL query dynamically, when the OP simply forgetting/having no idea at all about SQL syntax rules for strings, making their query like

SELECT * FROM t WHERE name = $name

habitually, these questions are tend to be closed with a suggestion to add quotes around a variable name.

The problem is, such a suggestion is promoting the most notorious vulnerability, making this code straight open to SQL injection.

So, in my opinion, the closure should be different, one which explains how to use prepared statements. However, there is a strong if not fierce opposition to such a closure. Their argument, as far as I can make it, "prepared statements are too complex a feature to grasp for a noob, so it cannot make an answer."

So the question is, should we avoid complex but secure suggestions in favor of insecure but easy to understand ones?

20
  • 13
    If I remember correctly it has been discussed on meta if we should write answers tailored for noobs and the results was: No, where are not responsible for the inabilities of an unknown reader.
    – Tom
    Commented Mar 20, 2017 at 12:57
  • This is a vaguely related question, but focuses on documentation: meta.stackoverflow.com/questions/332266/…
    – Tom
    Commented Mar 20, 2017 at 13:03
  • 3
    Closed with a suggestion to add quotes? So the close reason that people use is "simple typographical error"? Commented Mar 20, 2017 at 13:09
  • 39
    When that noob is, in a few years times, going to implement a feature or fix a bug in MINE online-banking software I would rather have that they are educated about SQL injection and other insecure practices, thank you very much.
    – rene
    Commented Mar 20, 2017 at 13:10
  • 5
    I guess the closest thing to a solution is to comment with that XKCD link. People prefer reading comics to reading Wikipedia entries.. so if you link that comic, the comment has a better chance of getting upvoted. Making it more visible. I'm afraid there's not much more we can do. Commented Mar 20, 2017 at 13:19
  • 1
    @S.L.Barth I am afraid so. The thing with Meta is that you always get contradicting suggestions and no certain answer at all. Commented Mar 20, 2017 at 13:20
  • 2
    Haven't you already covered a fair bit of this in your other meta question and your attempts there?
    – Jon Clements Mod
    Commented Mar 20, 2017 at 13:33
  • 11
    If the majority of the PHP community is fiercely opposed to quality content, then there isn't anything you can really do. If the community as a whole actually values quality responses, and actually has strong opposition to posting dangerous answers, there are lots of tools available through the site for the community to reflect that opinion and really shine a spotlight on the quality content. If the community really wants to shine that spotlight on bad content, posting a meta question on the subject isn't really going to change that.
    – Servy
    Commented Mar 20, 2017 at 13:33
  • 3
    @Servy's comment is spot on. That part of Stack Overflow really is a democracy. For better or worse. Individuals can only contribute their part (by downvoting and commenting) hoping that the consensus will eventually change.
    – Pekka
    Commented Mar 20, 2017 at 13:54
  • 1
    related: meta.stackoverflow.com/questions/344703/… Commented Mar 20, 2017 at 14:18
  • 1
    I know what you mean, I always want to slap the OP in the face when I see that type of queries, but I just letting them know about the risks of sql injection and recommend switching to prepared statements using MySQLi/PDO (In a comment), but I must say I never encountered anyone suggesting that prepared that statements are too complex feature to grasp for a noob
    – Alon Eitan
    Commented Mar 20, 2017 at 14:24
  • 2
    @Braiam - Does the duplicate have a prepared statement as one of the answers? This wouldn't be a quick fix but getting the secure answer on the dupe target would at least mean it's there.
    – BSMP
    Commented Mar 20, 2017 at 14:36
  • 2
    IMO, the answer should be the simplest ever possible, taking into account the minimum acceptable level of quality/security. "Disable your firewall" may be the simplest answer that fixes the problem, but it is not a feasible answer in a practical sense. Although for software questions it is often more of a gray area, I would argue that "add quotes" to the SQL question falls in the same category.
    – CompuChip
    Commented Mar 21, 2017 at 10:14
  • 4
    Everything should be as simple as it can be, but not simpler Commented Mar 21, 2017 at 14:29
  • 1
    @CompuChip nailed it. Where SQL is concerned, doing something like adding quotes to get it to work constitutes a "clever trick" that happens to get it to work, at the cost of causing major problems. Regardless of the PHP community's stance, SO overall is squarely in the camp of quality answers. It's not HackOverflow :) Commented Mar 21, 2017 at 14:52

3 Answers 3

57

should we avoid complex but secure suggestions in favor of insecure but easy to understand ones?

Short answer: NO! Of course not!!

If we're going to answer these questions, we should explain that this is dangerous. As @rene points out in a comment, in a few years this developer may be writing your banking application. Or they may be building medical software. Leaving them uninformed is not just a risk to them, but to society.

The problem is that it's a big step from using quotes properly to understanding SQL injection.
It may be necessary to explain the issue in two steps. First, showing how to use quotes properly. Then, to explain how this leaves the user open to SQL injection, and explain a bit about prepared statements.

And do link or insert that XKCD comic. It will help them understand the risk, much better than a dry text will.

3
  • 6
    "It may be necessary to explain the issue in two steps. First, showing how to use quotes properly. Then, to explain how this leaves the user open to SQL injection, and explain a bit about prepared statements." - which is exactly what their proposed duplicate target does.
    – CodeCaster
    Commented Mar 20, 2017 at 14:14
  • 2
    @CodeCaster I hadn't realized YCS meant this to be about a canonical question, as a duplicate target. It does explain why they discussed closing and a vulnerabilty in the same post. Commented Mar 20, 2017 at 14:35
  • I wish there was more incentive for people to provide a wiki answer because it would allow a fuller answer to be created; sure answer with a simple answer and avoid complexity, but allow others to come in later on to edit and add more complexity that fits the situation and educates the future further?
    – user9903
    Commented Mar 21, 2017 at 14:43
14

there is a strong if not fierce opposition to [closing such questions as duplicate of a canonical Q&A explaining the use of parameterized queries]. Their argument, as far as I can make it, "prepared statements are too complex a feature to grasp for a noob, so it cannot make an answer."

Given the daily influx of tens if not hundreds of questions by PHP beginners who forget quotes in queries and write applications open to SQL injection by hand-crafting SQL ("Oh, but it doesn't have to be secure, it's just a learning exercise"), we do need to cater for that scenario. It's been the most frequently found and abused security vulnerability for web applications for years, and it's so frigging trivial to fix.

If there actually is a bunch of users that don't want to help improve the site's credibility, its visitors knowledge and the web's security by pointing them to a succint source that explains what SQL injection is and how prepared statements / parameterized queries take away all the worries sprouting from building SQL queries from code, then those users do not have the best interest of all involved parties in mind.

The statement you put into their mouths is actively harmful for both the site and the visitors. If they really believe that, then the people claiming that must change their ways, or when they refuse to improve, be evicted from the site, maybe even with force.

4
  • He's not putting words into the mouth or anyone. There's one user that claims that every question that can be answered, should be answered, even if what they ask for is a widely known security vulnerability (like hashing passwords with md5).
    – Braiam
    Commented Mar 20, 2017 at 14:48
  • @Braiam that applies to the quoted text at the top of my answer, for which no source was provided, hence the "if".
    – CodeCaster
    Commented Mar 20, 2017 at 14:49
  • Well, I think he did so expressly so that it doesn't looks like a witch hunt/axe to grind against an user.
    – Braiam
    Commented Mar 20, 2017 at 14:52
  • 3
    @Braiam that's fine and all, but it doesn't change my answer. Look, the point here is actually that such questions get asked very frequently, and answered equally frequently with the quick fix of "add quotes in the right places". We don't need a witch hunt, we need a thorough solution, which is to dupe-vote every such question and educate all frequent visitors of said tags.
    – CodeCaster
    Commented Mar 20, 2017 at 14:56
2

Stack Overflow is a question and answer site for professional and enthusiast programmers.

Is it professional to give an answer which violates the most basic security rules? I think not, and I hope the community agrees on this.

This answer is short but I hope it met the standards of quality.

For the history: I started with PHP, I did not used prepared statements back then, but used mysql_escape_entities and htmlentities, which was already a good start. I used them because the online tutorial where I taught myself was careful to point that out from the very beginning. If it wouldn't have, I wouldn't have known.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .