96

TL;DR. A question with a title that asks specifically for crypto code, all answerers think it's reasonable to ignore security feedback, specifically because the question is poorly worded. Years in the making now this question/answers set is clearly propagating vulnerable C# code across Github. What can be done?

I think there is a better duplicate, others may disagree. Alternatively, I think it would be reasonable to change the title to match the answerers interpretation, not sure if OP would agree. Other solution? I think there is probably a broader discussion of security related answers on SO.


update: this specific example question has been adjusted in title and several of the high voted answers/proliferating bad copy 'n paste code samples are now no longer compiler ready (without minor editing).


full discussion:

There is this 8-year old question, with 190K views and almost 400 votes, Simple insecure two-way "obfuscation" for C# (original title "Simple 2 way encryption for C#"), which is problematic, both in question and all of its highly voted answers. If you look in the many comments in questions and answers, myself and other security minded individuals have had a long history of trying to give feedback on it.

The body of the question is really poorly worded with additional stipulations in what the OP wants, as he says "not mission critical" he just wants to "keep honest people honest" and conversely "anything I wrote would be less than worthless [...] and make it trivial to crack" which I feel invalidates the first part, and certainly the first part also isn't conveyed in the title.

"Keeping honest people honest" has allowed answerers to justify uninformed answers that use crypto APIs just plainly incorrectly, or answers that are merely obfuscation, despite many users including myself trying to point out the flaws.

For example, the accepted answer uses a hardcoded Initialization Vector in CBC encryption. To use the API correctly the Initialization Vector is supposed to be unpredictable. If it is predictable, it leaks data and also the avenues for attack start increasing. It also wrongly suggests that this is okay as long as you use a website to generate some new random bytes to hardcode into your app (nope still predictable).

However if you go to Github, you can find 53 repos in Github that have that exact hardcoded IV. How many other usages of this vulnerable code out there that can't be found, because they change the IV hardcode as the answer suggested? How many are in private repos? How many are in other projects due to being hidden in libraries? This becomes a bit of a public safety issue, because people are just reading the title "Simple two-way encryption for C#" and looking at the high number of votes.

My first thought was marking it as a duplicate of Encrypt and decrypt a string which is also 8 years old and has 500k views and 500 votes. This question also doesn't have an accepted answer, However all of its highly rating answers are reasonable security wise. I also have a long history with this question, and have noticed answerers on this question actually correcting or removing their answers based on feedback, I myself added an answer 5 years ago (full disclosure).

I think one of the superficial issues that might hinder marking it as being a duplicate is that the question in Simple two-way encryption says encrypting data, while the other specifically says encrypting string. I personally think this is superficial, because despite not saying so, the OP is clearly asking for a method to encrypt a string: he mentions wanting something better than rot13, and the OP's accepted answer only encrypts strings. However others may validly disagree.

My other thought, maybe if all the answerers of "Simple two-way encryption for C#" are resistant to fixing their objectively improper crypto constructions because the question stipulates "keeping honest people honest", that maybe this question could merely be titled "Simple two-way obfuscation for C#"; it would make everything more clear. The body of the question doesn't have to change; however, I don't feel like that's what the OP intended. So I'm not sure it would stay that way if I made that edit.

I've thought about flagging it for moderation, but I don't have a clue what to ask for in this case. I think this issue has reached this state due to the difficulty that most developers don't have expertise in security related programming, and don't have any idea how different it is. I don't feel like bad answers generally propagate to this great a degree on Stack Overflow, but maybe other people have different impressions on that as well.

I'd be really interested in other people's thoughts. I think that dealing with this question and its low quality answers is actually quite an important C# community issue, particularly due to the copy'n'paste propagation becoming a matter of public safety. Also maybe other people have experienced similar security issues in other language tags.

18
  • 29
    What about getting some people together (for example in chat), then asking a new, properly worded question, providing a new, well-thought answer, and then closing the old, bad Q&A as duplicate of the new, canoncial one? You could even close the other encryption question as duplicate of that one as well.
    – Polygnome
    Commented May 9, 2017 at 7:57
  • 11
    @Poly that doesn't make the insecure answers disappear from web searches. Why click a duplicate link when you can copy-paste code from an answer that appears to work and doesn't require user configuration?
    – CodeCaster
    Commented May 9, 2017 at 8:27
  • 4
    @CodeCaster You will not get the old code to disapper. Stack Overflow / Stack Exchange doesn't censor, this information will not simply disappear from the web. You can, however, make an effort to put the right information in the readers hands. Furthermore, you might be able to edit a big, fat disclaimer about the security issues into the upvoted answer and have it protected by a mod.
    – Polygnome
    Commented May 9, 2017 at 8:29
  • 5
    @Poly except they don't do that. The current rules are specifically crafted to result in the least possible amount of friction for people posting answers, and don't cater for security, architecture or best practices, because collectively maintaining that is hard. Votes should reflect a posts quality, except they don't. Again, not saying I agree, just reporting how it is.
    – CodeCaster
    Commented May 9, 2017 at 8:37
  • Related meta.stackoverflow.com/q/293930/792066
    – Braiam
    Commented May 9, 2017 at 10:28
  • 2
    Related as far as intent/impact: Fixing answers that recommend chmod 777
    – jscs
    Commented May 9, 2017 at 12:46
  • 5
    I tried to read this entire wall of text; honestly I did, but it sounds like your problem is that bad programmers program badly. Given the choice between lazily copying and pasting code or actually understanding what their code does, these programmers opt for the former. It's a problem, but not one Stack Overflow is set up to solve. Commented May 9, 2017 at 15:16
  • 3
    @MikeMcCaughan at least SO shouldn't make it easier/enabling these programmers to program badly... if anything, we should either provide the information in a way where the programmer is forced to understand what's going on or at least using the sane default.
    – Braiam
    Commented May 9, 2017 at 17:39
  • 3
    @Braiam I don't think SO is making it easier. SO provides a platform for answers of all kinds. Voting is supposed to be the mechanism by which good and bad answers are separated. The fact that voting allows a bunch of ignorant people to vote up an ignorant answer is part of the deal you get with voting mechanisms (cough*2016 US Electioncough*). Commented May 9, 2017 at 19:48
  • 2
    @MikeMcCaughan there wasn't an 400% landslide in the U.S. Election. There is some evidence SO is making this easier/enabling in these cases, however I don't think there is, or even should be, a way to force people to understand the consequences either. Guide them better on the other hand, probably would be good.
    – jbtule
    Commented May 9, 2017 at 21:17
  • 2
    Perfect example that votes mean nothing. Possible because upvotes are free while downvotes are not. Commented May 10, 2017 at 0:01
  • Maybe this could turn into a feature request, a "Flag as deprecated" button on questions and answers. After a certain number of flags a very obvious banner will appear at the top of the question or answer that was flagged explaining that it was correct at time of posting but is no longer the case. Users over a certain rep could comment on the banner perhaps with an explanation or a link to an updated or more correct q/a
    – Darren H
    Commented May 10, 2017 at 17:29
  • 1
    @DarrenH that was not the case here.
    – jbtule
    Commented May 10, 2017 at 17:32
  • 2
    I'm the original poster. If someone would post a good, valid answer that is secure (since you say the answer I marked as accepted is not), then I could see my way to possibly making the "accepted" answer the one you post. In fact, I did this very thing years ago -- I originally marked 1 answer as accepted, but came back a few years later and marked a different one the accepted answer based on the feedback in the comments.
    – Matt Dawdy
    Commented May 11, 2017 at 20:50
  • 1
    Then perhaps you should edit the title, question, and answer and state clearly at the top that no answers to this question should be used, and point the user to your preferred question and answer.
    – Matt Dawdy
    Commented May 11, 2017 at 21:23

6 Answers 6

37

I think some judicious editing would go a long way here.

My first suggestion would be editing the question, especially the title. If you re-paint the search target so that it's square and turquoise instead of nice concentric red and white circles, it should give some people pause before they shoot.

The body's reasonably clear that we're talking about keep-your-sibling-out-of-your-secret-diary-level encryption, but as you've aptly demonstrated, people ain't reading the body.

A title that actually made the point that none of the replies should be considered real security would be a good step towards making Google-inbound readers think twice about copy-pasting into their codebase.

This could be as simple as adding a mention of "insecure":

Simple insecure two-way encryption for C

but it might be better to move the title even further away from its current state:

Hiding unimportant data with built-in C# encryption

or

Making some data hard to read with built-in C# encryption

You might even consider not using the word "encryption" at all, although I can't think of a ready substitute. -- "obfuscation" was suggested in the comments.


Second, to your concern about the accepted answer and the hardcoded key/IV, I'd strongly suggest removing the values. If you really don't want people to open a door without considering that there's a snake inside, hanging a sign is not enough: you've got to cover the handle with the sign so people can't even grab the thing without touching the sign.

Swap in non-compiling placeholders, and completely eliminate the ability to use this answer as another box of Uncle Ben's 5-minute copypasta. This should force at least a handful of people to actually read the bit about using a random IV, increasing the chances that the right thing happens. At worst, you'll have fixed a readymade sample of bad practice in our corner of the web.

10
  • 2
    I'd go with your last propose title, but just removing "with build in C# encryption" from the end. It's gets across the crux of the question (making some content hard to read), the statement that the OP wants to use built in methods is something that can be in the body; that doesn't need to be in the title, and the use of the term "encryption" just isn't really appropriate at all, given that the OP doesn't actually want to implement a proper encryption protocol.
    – Servy
    Commented May 9, 2017 at 13:44
  • 2
    "A title that actually made the point that none of the replies should be considered real security would be a good step towards making Google-inbound readers think twice about copy-pasting into their codebase" - you have high expectations of those people. They will type "c# encrypt string" into their web search, and will end up on that question regardless the title (because of the phrases and code used in its answers), and wil still copy-paste the first code block from the first answer they see.
    – CodeCaster
    Commented May 9, 2017 at 13:47
  • Removing hardcoded values is a very good point, it makes them less copy-pasteable, putting some explanation of the pitfalls in code comments, might force some people to notice problems. @CodeCaster has a point as well on the title, I still favor swapping out the language encryption to obfuscation, but your suggested titles might be a good enough compromise.
    – jbtule
    Commented May 9, 2017 at 14:09
  • 1
    I think using "obfuscation" would be a great move, @jbtule
    – jscs
    Commented May 9, 2017 at 14:31
  • 3
    @CodeCaster: I don't disagree, but even if we save only one person from adding to that collection of 53 copy-paste repos, it's worth a minute spent editing.
    – jscs
    Commented May 9, 2017 at 14:37
  • 10
    @CodeCaster Page titles are weighted pretty heavily. There's a very real chance that removing encryption from the title will push it down on google's search results (at least low enough that it's not one of the top results, because let's be honest, people don't look past the first page).
    – Servy
    Commented May 9, 2017 at 14:58
  • 1
    FWIW, encryption that's easily crackable is called encoding :-) I actually like this idea of changing the question to make it clear security is a low concern (and possibly creating a similar one specifically asking only for secure methods. As one of the people pulled up by jbtule, I specifically edited my answer to make the shortcomings even clearer than they were (and I believe they were clear) but, if people then choose to use that code inappropriately, that's really their fault.
    – paxdiablo
    Commented May 10, 2017 at 1:06
  • 1
    I added some non-compiling placeholders to the top two aggregiously constructed aes answers as well. I'll probably spend some time this week posting issues to those github repos.
    – jbtule
    Commented May 10, 2017 at 4:14
  • 2
    Kudos, @jbtule; I rather think you could go a bit further still. I imagined completely removing the arrays for the key and IV, leaving just a code comment like "See description for how to generate these".
    – jscs
    Commented May 10, 2017 at 13:18
  • @JoshCaswell I don't want to fiddle with those AES answers too much, as they are still bad answers and I'm not really improving them, just trying to turn a tide of ignorant use. If someone else wants to go farther, where ever that leads, more power to them.
    – jbtule
    Commented May 10, 2017 at 20:25
22

Even if the answers were justified by the poor wording of the question, it seems to me that would just be reason to improve both the question and the answer(s). The purpose of Stack Overflow isn't to answer particular questions that particular askers have, but to create a resource of good questions and answers. Admittedly, that often gets lost in the day-to-day fray of answering individual questions, but a question on crypto that has bad (insecure) answers seems like the perfect example of a case where SO would want to do better.

I would argue for improving the question so that what it asks for is in line with good security guidelines, or finding a good duplicate and closing the bad question as a dupe.

Of course, it is also necessary that whichever question remains has a good, comprehensive answer that satisfies both the question and good security guidelines.

In short, I agree with the direction you are working towards on this question.

19
  • 2
    It's not clear to me what exactly you're suggesting in this answer. If you edit the question we're talking about, the security issues in the answers already given don't disappear.
    – CodeCaster
    Commented May 9, 2017 at 7:51
  • @CodeCaster Of course not, but the question should be improved so that it no longer contemplates insecure answers. Then, the insecure answers would clearly be wrong. Part of the problem is that people are defending the bad answers because the question allows for them. The other part of the problem is the bad answers themselves. Commented May 9, 2017 at 8:00
  • 2
    Well the question currently already reads like "I really don't care about security, how can I obfuscate some data that I don't care for being decrypted by a persistent user?" to me. If you edit that, you invalidate (almost) all answers, which is not allowed for an edit.
    – CodeCaster
    Commented May 9, 2017 at 8:01
  • 1
    @CodeCaster That makes it a bad question. If it can't be edited, it should be deleted. Commented May 9, 2017 at 8:10
  • 3
    That makes it a bad question security-wise, but not regarding the Stack Overflow standards. When it was asked, it was apparently fine to ask a question like that, and we generally don't go and back-date current standards on older questions.
    – CodeCaster
    Commented May 9, 2017 at 8:11
  • @CodeCaster Do you have a mouse in your pocket? Commented May 9, 2017 at 8:13
  • I mean "we" as in "we, the community".
    – CodeCaster
    Commented May 9, 2017 at 8:14
  • 9
    @CodeCaster we generally don't go and back-date current standards on older questions Sure we do. You moderate the site based on the current standards, not the standards of when a question is asked. The only exception to this is historically locked content, which has very strict requirements for getting an exception. The real problem is the first part of your comment, that the question, even now, doesn't actually violate any SE rules that would merit closure or deletion, it just asks for something that's a bad idea.
    – Servy
    Commented May 9, 2017 at 13:38
  • @Servy Depending on the context in which it's used, it's not even necessarily a "bad idea". A janitor's closet door doesn't need to be 8-inch thick steel with a timed, biometric lock. An "Authorized Personnel Only" sign is generally sufficient for the purpose of keeping folks looking for the bathroom out of the cleaning chemicals.
    – jmbpiano
    Commented May 11, 2017 at 17:40
  • @jmbpiano You're correct that not everything always needs the highest of security. A janitor's closet doesn't need time biometric locks, it just needs a sign. So if someone's asking to secure a janitor's closet you tell them to get a sign, you don't suggest that they go get times biometric locks, but just configure them so that anyone's fingerprint opens them and the times on the locks are open all day. Just having a simple security system that's not designed to protect against sophisticated attackers is fine, what's bad is having a sophisticated defense system that you implement wrong.
    – Servy
    Commented May 11, 2017 at 17:44
  • @jmbpiano The latter is actually much worse than just having a sign, because it gives you a false sense of security. It gives you the impression that you're safer than you actually are.
    – Servy
    Commented May 11, 2017 at 17:45
  • @Servy Agreed entirely. In this case, it appears someone asked how best to secure a sign on the door. Meanwhile, the responses were all "Here's how to set up a state-of-the-art biometric lock... that has a backup keypad with a default password of 1234". The problem in this case does not lie with the question itself.
    – jmbpiano
    Commented May 11, 2017 at 17:49
  • @jmbpiano No, that's not analogous. The question was asking how to set up a biometric lock, and said that they don't care if it actually works or not, they just want one that looks like it works. Someone then posted an answer of how to set up a biometric lock that doesn't actually work at all and lets anyone in, and someone else showed how to actually set one up correctly. Nobody provided a solution that simply doesn't use encryption at all (which would be the sign in our analogy). The problem does lie with the question, because it's specifically asking for this broken solution.
    – Servy
    Commented May 11, 2017 at 17:52
  • @Servy I disagree. I think it's perfectly reasonable to request such a solution. Just because someone does not own a dog doesn't make posting a "Beware of Dog" sign an unreasonable solution for discouraging solicitors. It's only "broken" when someone uses one as their only defense against thieves. The fault is primarily with the answers for not understanding that the solutions they were providing were insecure, not with the request itself. The question itself was quite clear on its (lack of) security requirements. Only the title was misleading and that's been (quite rightly) corrected.
    – jmbpiano
    Commented May 11, 2017 at 18:54
  • @jmbpiano You think it's perfectly reasonable to request instructions on how to install state of the art, top of the line, security system, but install them in such a way that they don't actually work, and are trivially subverted by even primitive attacks? Why do you think that's reasonable? If you don't want to defend against sophisticated attacks then you simply use primitive security systems, you don't use sophisticated defences in a broken way. Again, the answers aren't suggesting using a simple or primitive defense, they're suggesting using a sophisticated defence in a broken way.
    – Servy
    Commented May 11, 2017 at 18:58
17

Related:

The consensus:

  • Don't edit code in other people's answers. Or do.
  • Don't edit "This is bad code!" banners into answers, but feel free to add that as a comment and hope someone reads it before copy-pasting the code from the answer.
  • If someone copy-pastes insecure code without understanding it, it's their problem.
  • Post your own, better answer, and hope it rises to the top.

In conclusion:

It's fine to have answers promoting insecure or broken code. I mean, most code in answers is broken in one way or another, no reason to add exceptions to security-based questions.

9
  • 15
    It really isn't fine. Commented May 9, 2017 at 8:10
  • 2
    @David if you click the first link and read my answer, you'll realise I also don't like having insecure code on the site, just like you. It's just that according to the current rules, there's nothing we can do other than post our own answer and hope for the best. I don't see why we should add exceptions to that just for security-related questions, I'd rather change the quality standards altogether.
    – CodeCaster
    Commented May 9, 2017 at 8:13
  • 5
    If someone copy-pastes insecure code without understanding it, it becomes the user's problem. It's an important distinction. Both you and I might be one of the users of someone who used that piece of code. Buggy code is mostly the developer's problem, so it might be fine to leave broken code here and there. Security related stuff has a tendency not to show up until it affects real people; people that don't know, and aren't expected to know how to properly encrypt something.
    – Malt
    Commented May 9, 2017 at 17:30
  • 1
    @Malt I don't know what point you're trying to make. What are you proposing? That answers with security flaws shouldn't be on the site? Do note that in this answer I'm merely summarizing earlier discussions about this very subject; I've posted an answer in the first of the linked questions if you want to read more about my stance on the subject.
    – CodeCaster
    Commented May 9, 2017 at 17:30
  • 2
    @CodeCaster I can't suggest "not having them on the site" because that's impractical. But handling at least the popular questions that contain glaring security issues is easy. We can edit them and simply add a prominent warning. Bonus points for a link to a more secure implementation. Ideally, maybe we should have some flagging system that would change the answer's background or add a colored border around it if enough reputable people claim that the code is insecure.
    – Malt
    Commented May 9, 2017 at 17:42
  • 1
    @Malt banners have been proposed before, and are a solution. Maybe a better solution than doing nothing. It still has its drawbacks and things that need to be figured out: who is going to place such banners, and on whose authority? On which topics exactly? Does a highly-voted answer not bearing such a banner mean it is secure, correct or whatever we're bannering for? What if an unbannered answer gets outdated, or a bannered one updated? It's going to be a maintenance hell while not offering much in return.
    – CodeCaster
    Commented May 9, 2017 at 17:47
  • 4
    @CodeCaster I've now read your answer in the first link. I understand your argument, and it's not without merit. However clearly, down-voting isn't enough. Similarly to XKCD's ToradoGuard, we too can't always rely on averages and popularity. Who's going to place such banners? Similarly to "close" votes - reputable users, maybe ones with certain bronze/silver badges in the are. Does a highly-voted answer without the banner secure? No. But let's at least make the ones that aren't, especially if they are popular.
    – Malt
    Commented May 9, 2017 at 17:51
  • 1
    @CodeCaster Also, note that finding many of these answers is fairly easy. Just turn over the obvious stones - tags and keywords such as Encryption, Authentication, Validation would be a good start. Yes, it won't catch all code out there that's vulnerable to SQL injection of buffer overflow, but I think that if we at least do the obvious stuff, we should do it. Half a solution is better than no solution at all.
    – Malt
    Commented May 9, 2017 at 17:54
  • @CodeCaster The problem with the idea of letting anyone change anyone's answer when they think it's wrong/bad is that it works both ways. If you post an answer to a question like this in which you implement the security correctly, and ensure the system is actually safe, someone else could come along and "improve" the answer and "simplify" it to remove "unnecessary" steps (since the question doesn't require the code to be secure). The real problem in this particular situation is that way more people wanted the insecure solution over the secure one; there's no way to fix that.
    – Servy
    Commented May 11, 2017 at 19:03
13

I would argue that keeping posts with bad advice around is really harmful, at any rate. The question and its answers have to go.

I propose the following way of achieving this:

  1. Get some crypto-knowledgeable people to get together in chat and work up a rework of that question + an answer that meets today's security standards.
  2. Close the old question as a dupe of the new one.
  3. Delete the old question.

That way

  • You get a clean slate and don't have to "outshine" the old posts with many upvotes.
  • No code has to be edited against any author's will.
  • No obstructive "banners" ala "this is bad advice" have to be inserted anywhere.
  • No dozens of answers have to be deleted, just one question.
  • Hardly anyone should lose their rep, since the question is many years old.
1
  • Also in favor of deleting stuff, and even the "temporarily valid" answers, because sooner or later the obsolete answers will outnumber the good ones. But as long as the voting power doesn't take in account the knowledge of the voter, we just have to hope that "popular" and "correct" are equal.
    – xenoid
    Commented May 11, 2017 at 12:08
5

Edit the title to say "Simple text or data obfuscation for C#", and edit the body accordingly, e.g. "I don't really need encryption; I just want obfuscation stronger than ROT13." Bonus points if you add a paragraph describing the difference between the two.

0

My personal choice would be editing the question and the answers to put a notification expressing that this question or answer(s) are have major security issues and would be avoided if security is required on the related code.

Also some non-vulnerable SO Q&A links about this issue could be placed to the question so wanderers would visit them if they need a secure approach.

The question and the answers should stay as is so it would also be useful for lerners to see the way how to write vulnerable code so they will also learn what to conider in encryption.

Even the bad questions and answers have something to teach.

3
  • 2
    Yes a warning inside a quote tag (it renders with a yellow background) like I've done for example here is a good way to notify hurried programmers who otherwise are going to copy and paste the code and propagate the bad practice. Also, in the warning don't just say "Don't do that!!!1", but also provide a link showing how to do it correctly.
    – laurent
    Commented May 9, 2017 at 10:13
  • 7
    Please don't abuse quote markup as a "banner" just because it renders with a particular background.
    – jscs
    Commented May 9, 2017 at 13:22
  • 1
    @Josh it's how duplicate votes came to life as well.
    – CodeCaster
    Commented May 9, 2017 at 18:03

You must log in to answer this question.

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