What is meta? ×
Meta Stack Exchange is where users like you discuss bugs, features, and support issues that affect the software powering all 133 Stack Exchange communities.

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?

share|improve this question
7  
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 Jul 26 '12 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 Jul 26 '12 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 Jul 26 '12 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 Jul 26 '12 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 Jul 26 '12 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 Jul 26 '12 at 15:32
1  
@Waffelized Bobby I dont have to maintain a mysql database of WHAT HAVE YOU SAID EARLIER –  Aftnix Jul 26 '12 at 15:32
    
@mgilson changed the "Dictatorial" tone. It was wrong of me to chose words like that. –  Aftnix Jul 26 '12 at 15:36
    
@Aftnix -- and I removed my downvote (and am working on an "answer" which will hopefully add to the discussion) –  mgilson Jul 26 '12 at 15:36
    
Why limit this to just those cases? There's plenty of others to go around, e.g. printf(userstring);, strNthing. –  Flexo Jul 26 '12 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 Jul 26 '12 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 Jul 26 '12 at 17:16
1  
possible duplicate of [Is there a policy on dangerous answers? ](meta.stackexchange.com/questions/102536/…) –  Pops Jul 30 '12 at 14:23
    

1 Answer 1

up vote 5 down vote accepted

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.

share|improve this answer
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. –  Anna Lear Jul 26 '12 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 Jul 26 '12 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. –  Anna Lear Jul 26 '12 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 Jul 26 '12 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. –  Anna Lear Jul 26 '12 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 Jul 26 '12 at 18:14

You must log in to answer this question.

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