10

I've come across and helped lots of people with SQL INSERT queries, where nothing about the values being from the user and form input is being mentioned. The main point of the questions vary from simple syntax questions to how to insert using sub-queries, and so forth. The examples people use in their questions usually involve them writing the query in their programming language using variables, but nothing about getting these variables from user/form input implicitly or explicitly.

Yet, every time I write an answer, I am constantly getting criticized/downvoted for not protecting the query from SQL injection attacks. I mean a simple side-note is fine, but people tend to freak out, lash out, and downvote, even if my answer is correct and helpful. I'm getting sick of it, and want to see what the mainstream opinion of the community is regarding these specific scenarios.

Is it necessary for me to explain and example protecting against SQL injection every time I want to help someone with basic syntax problems when user/form input has nothing to do with the situation or question?

Improve this question
7
  • I've reopened it, but consider: if you strip it to its essence, you're left with "I am constantly getting criticized/downvoted...", "I'm getting sick of it, and..." and "how do I fix it?" which has been beaten to death on Meta. The answer is always the same: voting is anonymous, you'll never know why people will downvote, keep providing quality answers and your upvotes will compensate more than any number of downvotes can, etc. Commented Jan 21, 2012 at 2:41
  • 1
    @Shredder I think if you edit those portions to which casper is referring out, it would strengthen your arguments about the issue of how best to handle this type of situation in general, rather than focusing on how it might affect you. Commented Jan 21, 2012 at 2:55
  • @jonsca I agree (hence the +1 on the comment). Robert does a good job of addressing Shredder's points outside of the one's I felt are prominent, it would just would be nice to get the question a bit more out of the "I'm fed up with the rep system" realm. Commented Jan 21, 2012 at 3:06
  • @casperOne I definitely saw that negative component to it, so your move was totally justified, I just thought that the contention over these specific programming situations was worth a second look. Commented Jan 21, 2012 at 3:12
  • @jonsca Thanks for the justification, it's always nice to know. =) That said, it's getting that look, and it gives me a chance to voice some opinions on some of the negative (but not overwhelmingly so, it is what it is) aspects of SO. Commented Jan 21, 2012 at 3:14
  • 1
    Whoops who left this here? Commented Jan 23, 2012 at 15:19
  • This post should be migrated to meta.SO Commented Jul 19, 2014 at 22:11

5 Answers 5

Reset to default
17

Ultimately, there's a pedantry that permeates all topic areas at some point or another on Stack Overflow, and you're now experiencing it.

Sometimes it's born out of the right intention, someone has experienced deep pain points because of the barely-related issue. They want to spare others the same pain and genuinely help other people, even if it's not directly related to the topic at hand.

And then there are others that simply do it because they feel that they want to be "the most correct", or that they appear to be the smartest guy in the room.

Regardless, it sucks to be on the receiving end of it becomes it rarely comes off in a positive way (and, this is the internet, where nothing comes off in a positive way).

The result of this has its pros and cons; people who are unaware of these things will be enlightened, but those that are trying to focus on the issue at hand get penalized for not participating in the pedantry.

That said, I still believe part of my original comment is the best thing you can do; keep providing quality answers that focus on the topic at hand, the upvotes you receive will always outweigh the downvotes.

The system was designed intentionally to reward people who do this, you get +10 rep for receiving upvotes, and -2 for receiving downvotes.

That means for every person that believes you've provided quality content, five have to disagree. That ratio is huge.

In order for it to have any real impact, you'd have to provide something truly awful worthy of being downvoted into oblivion.

Improve this answer
13
  • 1
    -1, how am I supposed to craft any sort of reasonable response to this question when the things I'd say would look like they come from a "has to be correct," "pedantic" person that must be "the smartest guy in the room?" Commented Jan 21, 2012 at 4:56
  • 4
    @Charles The post makes no claim as to what the absolute source of the pedantry, only possible sources. It's the internet, it's everywhere and it's an attempt to elaborate to Shredder the reasons behind it here. Note, constructive criticism does not come off as pedantry. Commented Jan 21, 2012 at 5:41
  • 1
    +1, I caught myself engaging in this not too long ago. I didn't downvote, but I would leave comments about SQL injection just because they attracted comment upvotes. It's easy to get sucked into the pedantry. Commented Jan 21, 2012 at 6:39
  • I misunderstood your comments, but I see what you mean now. I guess I was looking for agree or disagree, black and white, answers, but I see your message and think it's a good answer. True; that is a huge ratio, and it is a good system. It is left up to individuals in the end, and I would just have to step up my game if I want to accommodate for that, or not give a crap.. Thx. Commented Jan 21, 2012 at 9:32
  • 7
    Pointing out SQL injection holes in people's answers is not pedantry. If a knowledgeable person posts code on SO, the implicit assumption is that "this is good code, use it". Commented Jan 21, 2012 at 12:55
  • 4
    @Pekka I respectfully disagree on both points. It depends on the topic at hand IMO. Also, SO is not Github, nor are we a production-level source code factory. It's great that people provide the quality that they do and I encourage that, but anyone simply copy-and-pasting without giving it a thorough review has bigger issues. It's still the Internet and Stack Overflow has a massive number of people contributing code, caveat emptor will always apply. Commented Jan 21, 2012 at 15:00
  • 2
    @casper I see your point, but still... People posting broken code in answers feels wrong. We are supposed to be experts to some degree, and people will use our code. The OP's situation is different than I though though - I misunderstood his question a bit. If there is no visible, definite SQL injection hole, downvoting because an answer fails to acknowledge their existence is terribly pedantic. So removing my -1 Commented Jan 21, 2012 at 16:56
  • 1
    @Pekka'sOrganicRepFarm How do you think moderating all that feels (since our job is to deal with the truly worst of the site)? I take multiple showers scrubbing with steel wool throughout the day and I still can't get the filth of the worst of the site off me =) Commented Jan 21, 2012 at 16:58
  • @casper hahahaha! Commented Jan 21, 2012 at 17:00
  • 2
    @Pekka'sOrganicRepFarm I agree that it feels wrong but sometimes, when the weight of a community comes down on you (ahem moi) it can be a little overwhelming to that person; especially when that person's intent is simply to do the right thing. Commented Jan 21, 2012 at 17:00
  • @casper yeah, I can see that. Commented Jan 21, 2012 at 17:01
  • 2
    Interesting fact: The "something truly awful worthy of being downvoted into oblivion" example you gave, at the time of this comment, has 92 downvotes and 34 upvotes. So even though it has been downvoted into oblivion, it still has so far netted the asker +156 reputation points. Not shabby at all for a single answer. For an example of a truly unpopular answer, I'd go with something more like this: meta.stackexchange.com/questions/915/… Commented Jan 26, 2012 at 21:17
  • @BenLee Done. I wasn't looking at meta when I was looking in the data explorer. Commented Jan 26, 2012 at 21:18
25

SQL injection is perhaps one of the most common mistakes in programming, and also sadly one of the most dangerous to them and their customer. It really is very serious, and in short: no I do not think we should miss any opportunity to educate people of this very real and very present risk. This isn't something like debating which JavaScript utility library to prefer, it is "your site, and all your data, are fundamentally at risk".

Yes, answer the question sticking on topic. But also yes: it is entirely correct and appropriate to note to the OP that there is a significant error in their approach.

Also keep in mind that this mistake won't just affect the code in question - it will undoubtably (in most cases) permeate their entire code base, and needs attention as soon as possible.

This is not pedantry. If we "answered" such a question without making this observation, we are perhaps even failing in our intent to help the user. A bit like helping them fix their home's windows while not commenting on the fact that we can see the kitchen is on fire.

Improve this answer
5
  • 1
    I don't disagree with any of this, but there's a right way to go about it, and a wrong way to go about it. Unfortunately, IMO, the users of SE tends to favor the latter (although we do have standouts from that). Commented Jan 21, 2012 at 17:26
  • 2
    @casperOne I fully agree that the "how" is important too Commented Jan 21, 2012 at 18:37
  • While there is no "significant error", I do think it is pedantry. There was no "observation" to make regarding values being from user input and there being a sql injection hole in my example. Aside from that, I agree that it is very serious and we should take the opportunity to at least mention it. Commented Jan 22, 2012 at 0:54
  • 1
    @Shredder Just one comment: When your argument starts to be "there was no indication it was from user input", you start to get into pedantry, yourself. Commented Jan 22, 2012 at 21:01
  • @AndrewBarber Yeah, this whole thing is dumb. I had a bad week and let it get the best of me. Wish I could delete all of this Commented Jan 23, 2012 at 17:59
15

tl;dr: Just use parameters in the code you post, and avoid the whole ugly ordeal.

It is what it is.

I've been on the other side, asking people about a simple concatenation SQL string, pointing out to them that this is an internal application, it is not exposed to the outside world in any way (via the internet or by distribution), will never see the light of day outside the office, is used by people who barely know what a keyboard is, and therefore will never, ever be vulnerable to a SQL injection vulnerability.

No dice.

You're going to get flak for this, no matter what you say. If you're willing to accept that flak, go ahead and post code with injection vulnerabilities. There will be more than enough people willing to point out what is wrong with it, so you don't have to. If you're not willing to accept the flak, use parameters in your SQL statements, and make it a teaching moment for the OP.

See Also
What's the most useful thing to say/link to when a user doesn't escape inputs in their example? (or other common pitfalls)

Improve this answer
7
  • 5
    "used by people who barely know what a keyboard is, and therefore will never, ever be vulnerable to a SQL injection vulnerability". Hah! I would hope that was a joke; But, I've seen enough of companies' internal systems "hacked" to know better. I suppose it's a blessing that those departments that try to compartmentalize information -- to the detriment of the company -- are the least aware (at the management level) of basic security. Digital karma at its best. ... Any system is way, way more vulnerable to insider breaches than it ever is to outside attack. Commented Jan 21, 2012 at 7:27
  • Sorry, I've had a bad day. What is your answer to my question? I am obviously under the impression that I'm going to get flak for this. Commented Jan 21, 2012 at 9:28
  • @Shredder: I updated my answer. Commented Jan 21, 2012 at 15:48
  • 1
    Where are there injection vulnerabilities in my code? That's the point I'm trying to make. There is no need to protect, because the code is not evidently showing values coming from user input. Especially in my example code, where I'm setting the values myself! Commented Jan 21, 2012 at 16:20
  • 1
    @Shredder: That won't matter to the pedants. Commented Jan 21, 2012 at 16:27
  • well, im glad the majority sees this as pedantry (makes me feel better hehe><;). I've gotten my answer (i do need to just "accept" the things that will take place), and really appreciate the effort everyone put in to this question. Commented Jan 22, 2012 at 0:49
  • 1
    "pointing out to them that this is an internal application..." I created a little app that sat in the taskbar that people in the office could use to coordinate breaks. The office jagov jumped on the machine of one of the people that had it installed and started spamming the F out of it, because it was an "internal" app not facing outside and so I didn't prevent people from spamming 1000 "break time!" messages. Commented Jan 23, 2012 at 15:25
12

Is it necessary for me to explain and example protecting against SQL injection every time I want to help someone with basic syntax problems when user/form input has nothing to do with the situation or question?

If you mean whether you should protect your own code that you post from SQL injection, then yes. Code that you show will be copy+pasted. If not by the OP, then maybe by someone else. The very nature of Stack Overflow as a community resource built for the ages dictates that any code you post as an answer, should be clean from obvious security problems.

If I see something like

 mysql_query("SELECT * FROM '".$_POST["id"]."'");

in an answer, I will downvote it, or at least comment on it.

What I find wrong is downvoting answers that answer the question, but fail to mention a SQL injection hole that is visible in the OP's code (but is not repeated in the answer). But I've never seen that happen before.

Improve this answer
3
  • I see, but the code that I posted (in my example) did not need to be protected from sql injection. Neither was there any evidence in the OP that his values were from user input, so no evident sql injection hole. That's the point I'm trying to make. If I had code like you have, I would expect a downvote. Commented Jan 21, 2012 at 15:45
  • @Shredder ah, I see now that I slightly misunderstood your question. Yes, when there is no injection hole visible, I agree that downvoting an answer that fails to mention their existence is pedantry. Commented Jan 21, 2012 at 16:54
  • that's good to know. I feel a little saner, ty Commented Jan 22, 2012 at 0:32
2

Your answer will hopefully be used by people other than the person asking the question, and as you gain rep your answers (particularly accepted answers with upvotes), gain more weight -- they seem more correct. If an answer is dangerous in the general case, and you don't highlight that fact, it is a good thing for others to do so.

Sql injection is not the same thing as using a singleton, it's not just usually a bad idea, it is always a bad idea -- even in your own code where the input does not come from a user. The code may be refactored to be more generic and then used elsewhere.

I would suggest that you take this as constructive criticism, rewrite your answer, and say so in a comment, thanking the person that gave you the criticism.

Improve this answer
1
  • Even if the input is not user input, there could still be reserved characters in it that parameterization would solve. +1 Commented Jan 22, 2012 at 21:03

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.