23

I read this How to get the client IP address in PHP? answers, but in Tim Kennedy's answer the code has a security issue.

The user doesn't necessarily read all sentences of the message and comments when he needs to a quick fix, particularly if it's a big message.

I made this edit, and I "highlighted" the first idea on the security vulnerability.

I don't understand why my edit was rejected. Why does the code with a vulnerability not have a comment on the security issue?

I thought that Stack Exchange highlights the safety and right code.

10
  • 5
    The problem is that the reviewers are usually not experts in the topic of the question/answer, so they often reject edits that require reviewing of technical content, instead of skipping.
    – user000001
    Commented Jul 20, 2014 at 9:54
  • What do I to do ? Nothing ? Commented Jul 20, 2014 at 11:10
  • Well there is a highly upvoted comment that warns against using the code. If you really wan't to edit it you will have to wait until you earn 2000 points.
    – user000001
    Commented Jul 20, 2014 at 11:13
  • 1
    Thank but it's not important to me this edit, but I want help the beginners who use this code without read the answer text. Commented Jul 20, 2014 at 11:20
  • 2
    Point it out with votes and comments (the comment is already taken care of). It’s not your call on whether someone else’s answer should decry itself.
    – Ry- Mod
    Commented Jul 20, 2014 at 14:34
  • 24
    @minitech the problem with that is that clueless noobs will highly upvote anything that at first sight looks like it will solve their problem, while they have no freaking clue and understanding about the security issues. You can leave an "Editors note: security vulnerability" to make it clear where the original author's words end and the editor's begins. Warnings in comments are not enough sometimes, especially when they're not bold, and when they're buried beneath a pile of other comments.
    – user456814
    Commented Jul 20, 2014 at 19:00
  • 5
    I like your edit suggestion and would have voted to accept it - but then, people here have a point, too. Ultimately, it's the programmer's responsibility to understand the code they are using, not ours. Especially when everything is explained in the same answer, in full.
    – Pekka
    Commented Jul 20, 2014 at 19:27
  • 5
    @Pekka웃 I would rather not have noobs running around punching security holes into software that I may end up using eventually .
    – user456814
    Commented Jul 20, 2014 at 19:28
  • 5
    @Cupcake If they copy&paste code without even reading the text right above it, there's nothing you can do to save them and they're going to create security holes either way.
    – Pekka
    Commented Jul 20, 2014 at 19:29
  • 1
    Without say "noob". Lot of people don't have the English mother tongue... And don't read the big block of the text who isn't very well presented like me. Commented Jul 20, 2014 at 19:49

2 Answers 2

26

Editing other people's answers is a complicated subject, and there are various stances about how to go about it, and whether it's even appropriate at all:

  1. Good question, old version-dependent answer
  2. Is it OK to edit answers to show that they are obsolete, out-of-date, and deprecated?
  3. How do I deal with answers that are good for experienced users but potentially dangerous for newbies?
  4. Should I accept an edit that adds information to an answer?

I've stated before that I generally prefer not to change the meaning of other people's answers (though I will make the occasional exception every now and then, e.g. there are upvoted comments to the author to make a useful change).

One of those exceptions is with security issues. For example, there are (highly upvoted) answers that describe disabling SSL verification while using Git, which can enable someone to impersonate a trusted endpoint (e.g. GitHub, or any other remote Git host).

I've edited warnings about the security implications into the answers, and made it clear that the warnings were not part of the original author's answer by explicitly stating that they were left by an editor:

Editor's note: disabling SSL verification has security implications. Without verification of the authenticity of SSL/HTTPS connections, a malicious attacker can impersonate a trusted endpoint (such as GitHub or some other remote Git host), and you'll be vulnerable to a Man-in-the-Middle Attack. Be sure you fully understand the security issues before using this as a solution.

So my point is, if you're going to make a drastic change to someone else's answer to highlight security issues, make it clear that you've done so by stating that it was an editor that highlighted the issues, not the original author.

Of course, making a drastic change like this is easier when you have full-edit privileges. If your edits have to go through peer review, I can see such edit attempts as being shot down by reviewers who generally hate drastic edits slipping through because of robo reviewers.

Couldn't you leave a comment instead of editing?

Sometimes, yes, you can leave a big bold comment, especially on answers that don't already have other comments (or have very few):

WARNING: please pay attention to the following security issues! Blah blah blah..

However, sometimes people don't make their warnings in the comments bold, so they don't stand out, even if they end up being highly upvoted:

Sceenshot 1

Further more, sometimes no one notices security issues until much later, when an answer has already accumulated a lot of comments, so any warning comment you leave just ends up being hidden by the other previous (upvoted) comments...so no one generally sees the warning.

5
  • Thank for your answer. It is hard to send edit with the peer review, except when I add 3 spaces to "Improve formatting"... Commented Jul 20, 2014 at 20:26
  • @HorsSujet the suggested edit reviews have known problems. If you think you might have trouble making an important security edit in the future, bring the issue up on Meta again.
    – user456814
    Commented Jul 20, 2014 at 20:30
  • 11
    That SSL edit is a good idea. It's a bit disappointing that some of the most highly upvoted answers on the ssl tag (I mean those with silly numbers in the 100+) are precisely those that recommend to disable certificate verification, just to make the error message go away...
    – Bruno
    Commented Jul 21, 2014 at 0:03
  • @Cupcake What I do if all answers duplicate a security issue ? Like this (I edited just the first). Commented Jul 21, 2014 at 19:29
  • @HorsSujet I would go ahead and add a notice to all of them, but that's going to be tough to do if you don't have full-edit privileges. Maybe ask for someone's help in the PHP chat room? Otherwise, I guess you'll need to trust the reviewers to approve your edits.
    – user456814
    Commented Jul 21, 2014 at 20:11
0

Some reviewers tend to repeat the decision of the first reviewer, rushing together to the "community conclusion" like a herd of bisons. Maybe we should hide the votes of other reviewers.

From the other side, maybe you have over-tried by also adding a comment into the source code and putting the text in bold. However the information itself is definitely useful and correct. If such edit has been rejected, add a comment instead.

You must log in to answer this question.

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