4

Almost all the code I see with tags in questions uses gets() or scanf(). It's okay that new coders get their input from stdin that way. But when someone fixes his code, in my opinion, he should change their gets() and scanf() calls to fgets() with sscanf(), or any other secure combination.

You may think that was not the original problem, so why should we change it? Well, their habit of using insecure coding will not be changed. And, in the future, all of a sudden, you will get hacked because one of these guys will someday write your router firmware or other system software.

You may say that when they are competent enough to write firmware, they will have also learned to avoid this. It's true to some extent. But I worked on a firmware project in my university years, and I had no idea about security bugs. I wrote my part. It shipped with a GPS tracker product. My company didn't have any code review practice. So no one noticed. My managers were incompetent enough to accept my svn commits happily.

Looking back, those bugs, which my code introduced, give me a quiet nightmare. I personally avoid the hardware products of the company I worked with.

The bottom line is this: if they were removed from stdlib, it would be better. But that's not happening because of existing code bases.

So my opinion is to fix code with security issues. Most of these fixes are trivial enough. Does anyone think the same thing?

Improve this question
15
  • 7
    Related? Ethics for SQL insert queries vulnerable to SQL injection
    – Time Traveling Bobby
    Commented Jul 26, 2012 at 15:26
  • 2
    That's no reason for a downvote. That's a reason for a comment at most. And don't go around fixing that whenever you see it. Leave a comment, but particularly don't change code sections.
    – Bart
    Commented Jul 26, 2012 at 15:26
  • In fixing code in questions might you not disguise the OPs ignorance of what is "best"? I don't know what is best btw.
    – ben is uǝq backwards
    Commented Jul 26, 2012 at 15:26
  • @mgilson its tagged as discussion. The topic is important. But its not suitable for SO. Now you are saying its not suitable for meta also. So i guess its a real downside of stack exchange that important issues like these has no place.
    – Aftnix
    Commented Jul 26, 2012 at 15:29
  • 10
    I've said it before, I'm going to say it again: DO NOT FIX CODE IN QUESTIONS!
    – Time Traveling Bobby
    Commented Jul 26, 2012 at 15:29
  • 3
    This is quite a valid discussion, but I don't think it is good to rampantly edit all questions/answers on SO to remove scanf() and gets(). It is a good idea, though, to edit canonical answer (answer only) on questions with high views to warn user about the security issue with using those functions.
    – nhahtdh
    Commented Jul 26, 2012 at 15:32
  • 3
    @Aftnix -- Yes it's tagged for discussion (which is good), but what do you want to discuss? As this post is, it seems pretty 1-sided. It reads like this -- "Here's my view, Here's why I'm right, Go do something about it". This would be made a lot better if you simply asked "What do you think we should do about users using these particular functions? Should we edit these functions out or post comments?". If you rephrase this in a way that it gives us something to discuss, I'll happily remove my downvote (and maybe even upvote as I think it raises some valid points).
    – mgilson
    Commented Jul 26, 2012 at 15:32
  • 1
    @Waffelized Bobby I dont have to maintain a mysql database of WHAT HAVE YOU SAID EARLIER
    – Aftnix
    Commented Jul 26, 2012 at 15:32
  • @mgilson changed the "Dictatorial" tone. It was wrong of me to chose words like that.
    – Aftnix
    Commented Jul 26, 2012 at 15:36
  • @Aftnix -- and I removed my downvote (and am working on an "answer" which will hopefully add to the discussion)
    – mgilson
    Commented Jul 26, 2012 at 15:36
  • Why limit this to just those cases? There's plenty of others to go around, e.g. printf(userstring);, strNthing.
    – Flexo - Save the data dump
    Commented Jul 26, 2012 at 16:38
  • 2
    -100 if you touch code in questions. -0 if you touch code in answers. +1 if you comment or answer in a helpful manner.
    – user7116
    Commented Jul 26, 2012 at 16:46
  • "Almost all the code I see with c tags in questions uses gets() or scanf()." Almost all, really? I see scanf fairly often, but I rarely see gets.
    – Keith Thompson
    Commented Jul 26, 2012 at 17:16
  • 1
    possible duplicate of [Is there a policy on dangerous answers? ](meta.stackexchange.com/questions/102536/…)
    – Pops
    Commented Jul 30, 2012 at 14:23
  • See also this answer by Shog and Technically valid answers that raise questions of morality.
    – Pops
    Commented Jul 30, 2012 at 14:45

1 Answer 1

Reset to default
6

I personally don't think that it is appropriate to edit the code in a question. This is something that the user is struggling with. It also can show the particular user's knowledge/experience of the language. It is, however, completely appropriate to leave a comment so the poster knows the error of their ways.

Answers are (possibly) a different story as the best answers should solve the problem using the best coding practices and should be edited. In this case, I usually leave a comment first. This notifies the poster so that they can learn something too (SO is all about learning after all). If they refuse to fix it, at least your comment is a flag for everyone to see. If you feel really strongly about it, you can go back and edit their answer yourself at a later time or answer the question yourself with an explanation that the particular functions used in the original question are unsafe.

Improve this answer
6
  • 6
    Leaving a comment and providing your own answer (with appropriately fixed/modified code) is probably a better way to go. You can also optionally downvote the answer you commented on if it's really egregiously bad. That said, you're absolutely right about not editing the code in the question. Doing so would potentially change or invalidate the question and that's bad.
    – Adam Lear StaffMod
    Commented Jul 26, 2012 at 15:57
  • @AnnaLear -- You might be right about adding your own answer, but if it's an old question/answer that has upvotes (and maybe even an accept), it will get much more visibility than your new answer (added possibly much later). Do you still promote not editing the solution (provided you don't change the content that is pertinent for the question)?
    – mgilson
    Commented Jul 26, 2012 at 16:03
  • 3
    Yeah, that's a bit trickier. I usually favour adding a new answer no matter what. It'll bump the question and ideally get new eyeballs/upvotes on your (better) answer. Combined with a comment on the original answer pointing out the problem, it should be visible enough.
    – Adam Lear StaffMod
    Commented Jul 26, 2012 at 16:05
  • 2
    @AnnaLear -- does bumping the question actually do much? How many people really sit there watching the main page on SO? I personally just watch the particular tags that I know I can answer/am interested in.
    – mgilson
    Commented Jul 26, 2012 at 16:08
  • It comes to the top of the "Active" list in every tag that's on it. I'm not sure exactly how visible it becomes as a result, though.
    – Adam Lear StaffMod
    Commented Jul 26, 2012 at 16:10
  • Good middle ground. But i'm up for editing the answer. Because, in many cases i have observed, people uses the code in answer verbatim. I've seen this practices in the new recruits we have in our team. When they are assigned small tasks, we see git patches without any uniformity of coding standard. I've personally found some of these codes go back to "fixed" code given in the answers in SO.
    – Aftnix
    Commented Jul 26, 2012 at 18:14

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