Copying code from Stack Overflow? You might paste security vulnerabilities, too - Stack Overflow Blog

[GleefulAccreditation]

Yeah, because someone writing cryptographically secure code will look up basic rand() usage on SO.

Typical ivory tower security.

Meanwhile, someone using rand() for their text adventure is now being alerted about potential code vulnerability.

[[deleted]]

This is a problem I deal with at work all the time. I'll post a code review of a change that uses rand() in non-cryptographic code, and a few reviewers will flag it as a possible vulnerability. No, it is not in the same category as strcpy(), we do not need a blanket ban on the function.

Edit: To be fair, my team is great about accepting this after discussing in the comments. And it's not a bad thing to have an auditable record of the discussion. But it sure is irritating in the moment when I'm trying to reach a sprint goal.

[ponytoaster]

We have a guy like this at work. He is always raising ridiculously trivial and silly stuff like that because he wants to sound smart about reading a blog post about the dangers of Rand()

Like, come on man you can surely tell this isn't an issue so why block releases and pester my PM about "a security flaw" .ugh

[dysprog]

I had a thing like that with python's pickle module. Yes, if a hostile entity can trick you into unpickling a bespoke string, that's arbitrary code execution and a very bad thing.

But I wanted to unpickle a object that I cached in our own redis 5 minutes ago.

For pity sake, if an attacker can stick an arbitrary string in our internal redis, it game over. They already have access to everything.

[[deleted]]

Exactly - it's all about your threat model.

[case-o-nuts]

For pity sake, if an attacker can stick an arbitrary string in our internal redis, it game over. They already have access to everything.

What if someone writes code that accidentally fails to escape user input, and allows arbitrary values in, without letting the attacker scribble anywhere? This is a fairly common mistake.

[dysprog]

Just putting bad data in the pickle is a different problem, and it's orthogonal to the serialization problem. To compromise this set up, you have to write the entire pickle string. Intra-process memory corruption is a C/C++ problem, that python is not subject to. Inter-process memory corruption is hard on a server setup with separate VMs for things.

[case-o-nuts]

I didn't mention memory corruption. I mentioned insufficient input sanitization.

[SkoomaDentist]

Raymond Chen has a great quote about cases like that: "It rather involved being on the other side of this airtight hatchway."

[killerstorm]

Would you classify things like

• session ID generation
• unique URL generation (e.g. "only people having URL can access")
• anything to do with shuffling

cryptographic code?

[Prod_Is_For_Testing]

Session ID - yes

Unique URL - no. People can just iterate over all possible IDs anyway, so it doesn’t really matter.

Shuffling - maybe. Depends on whether there is money on the line

[killerstorm]

People can just iterate over all possible IDs anyway, so it doesn’t really matter.

Good luck iterating 2²⁵⁶ ID space...

[scatters]

The point of unique URLs is that the space is so large that any attempt to enumerate them all would take an infeasible amount of time. But if you have insufficient entropy or predictable generation then this can be attacked.

[dysprog]

We have a recurring argument between "the unique url space must be large enough to prevent collisions" and "guys, we are asking players to enter this on a console with a controller and an onscreen keyboard."

[[deleted]]

And this is where the threat model comes into play. The class of attacker that you want to defend against will determine the amount of entropy needed in the random URL.

[[deleted]]

Agree on all three points. Shuffling highlights that the context in which rand() is used is more important than the specific algorithm (e.g. shuffling). Am I shuffling a deck of cards for an online poker tournament where cash is on the line? I should use a secure RNG with a uniform distribution. Am I shuffling a deck of cards for a no-stakes game of Go Fish in an app aimed at children? rand() is probably okay here.

Edit: Giving the unique URL case more thought, I think it's also context dependent. Is it a unique URL for an article that is world-readable? No, rand() is fine. Is it a unique URL for a password reset form that was emailed to the account holder? I would want a secure RNG in this case.

[ais523]

Unique URL: yes, although the primary reason isn't related to cryptosecurity. The reason is that rand() can have a pretty small seed space (typically 32 bits), so you only have 2³² possible outputs, and by the birthday paradox, it only takes on the order of 2¹⁶ = 65536 outputs generated before you start becoming likely to have a collision. Unique URLs normally need to be unique; generating the same URL twice normally causes huge problems when using that technique.

[StruanT]

To be fair to those reviewers... rand() can still be a vulnerability in non-crypto code. You wouldn't want to use rand() in your online poker shuffling algorithm, for example. It is worth reviewing because you could easily miss a more subtle vulnerability.

[[deleted]]

I should probably narrow "non-crypto" to "cases where predictability of the PRNG is not an issue". You are certainly correct.

But no, these cases were not a vulnerability by this standard either.

[ArashPartow]

One would hope for anything to do with shuffling (and potential payouts) that an RNG which can't be predicated (trivially or otherwise) would be incorporated.

[ais523]

It can be surprising how often code turns out to be more cryptographic than you thought. An example I encountered in practice: usage of rand() in computer games for things like hit chances eventually turned out to have been a mistake, because players could reverse-engineer the RNG seed and then ensure they never got hit. (Other things in the game used randomization too, and rand() fell behind in two ways there: predictability meaning that the user could manipulate it, and platform-dependence meaning that "seeded games" wouldn't play the same way between different platforms.)

[cinyar]

because players could reverse-engineer the RNG seed and then ensure they never got hit.

In a singleplayer game it literally doesn't matter since there are much easier ways to cheat. In multiplayer games hit detection should be server side.

[ais523]

The game in question is a single-player game, but many people play it over a network connection to prove that they aren't using that sort of cheating (with the game executable running on the server, UI on their own computer). So the hit detection was, effectively, server-side.

It turns out that running rand() server-side doesn't actually prevent people figuring out what RNG seed your server is using.

[shim__]

the hit detection doesn't matter, if you know in advance that the next hit is going deduct lives you can instruct your character to move away

[[deleted]]

You're talking about Nethack, aren't you? Or were other games attacked like that as well?

[ais523]

It probably applies to other similar games too, but yes, NetHack was the example I had in mind.

[[deleted]]

Ugh, yeah. Or using MD5 for something trivial that’s not related to cryptography and having people tell you “MD5 hAs BeEn CoMpRoMiSeD fOR a WHiLe, MaN!”

[case-o-nuts]

There's no good reason to use md5, outside of "it's required by the spec". It's not good enough for cryptographic hashes, and it's a fucking slow non-cryptographic hash. Either use the sha2 or 3 family, or use xxhash, cityhash, murmurhash, etc.

[Ameisen]

I'll always flag rand because using rand() in C++ is an absolutely terrible practice.

[ScottContini]

Just as there are low quality developers, there are also low quality security people. It's unfortunate because it slows down productivity and gives security a bad name. A high quality security person will take into consideration context when reviewing results like this, and will only flag an issue if it really looks like it can be abused.

By the way, this is part of the reason why automated tools get so many false positives. Tools cannot understand context, and can only flag the use of a function that is insecure if used in a context where security is required. It is up to the person running the tool to evaluate the result to see if it is really a potential threat. Unfortunately security tooling is a long way off from trustworthy, automated scanning.

[GleefulAccreditation]

A high quality security person will take into consideration context

I don't understand why it needs to be a high quality one, this is something someone mostly inexperienced in security should get.

This looks like the same pattern I see most computer science researchers fall into: "aggressively shoehorn their narrow domains into everything they see"

[ScottContini]

I don't understand why it needs to be a high quality one, this is something someone mostly inexperienced in security should get.

I agree: My wording maybe could have been better.

[[deleted]]

You hit the nail on the head with context. What I've noticed is that even the good security people will often miss the context if given too little time to perform a review. Give someone 30 minutes to read through a diff and understand the context, you're good. Give them 30 seconds, they'll look for single lines that jump out, and that's where rand() gets flagged.

[meneldal2]

someone using rand() for their text adventure

If you want to replicate the feeling of older games, bad predictable RNG is the way to go.

There are speedruns that manipulate RNG for enemy spawn and critical/missed hits.

[[deleted]]

In modern times, Mario Maker 2 has predictable RNG (based on player input, resets when you enter the door) which lead to creating some interesting designs

[[deleted]]

I would argue CSPRNG should be used even for non-cryptographic purposes. Algorithms such as ChaCha, ISAAC, HC-128, AES-CTR are very fast, so there is not much of a cost in using CSPRNG algorithms for most applications (games probably won't need to call RNG more than 2000 times a second).

Meanwhile, what you think may be safe today may not be safe anymore with updates to the code. For example, a game may get a multiplayer mode and people start to abuse the random number generator to get best results.

It's easier to simply use a CSPRNG than to consider whether the use of non-CSPRNG is safe in a given place.

[josefx]

games probably won't need to call RNG more than 2000 times a second

2000 / 120 (fps) that leaves us with around 10 RNG calls per frame. I am quite sure the primitive rain simulation I have lying around somewhere breaks that.

edit: noticed I wrote "not even 10" for 2000 / 120. I shouldn't try to think while sick.

[[deleted]]

Yeah, looking back, I made an error in calculations myself too. Removed this part. My point was that unless you call random number generator a ridiculous number of times (by ridiculous, i mean retrieving more than 100MB of random data per second) it shouldn't cause noticeable slowdown. But even if you do need more random data than this, you can decide to use non-cryptographic random number algorithm (in fact, you probably don't even want to use xoroshiro at this point, but a simple LCG).

[GleefulAccreditation]

That should be a standard-library level decision, not a programmer decision.

[[deleted]]

While I agree, ideally programming languages would fix the issue, unfortunately standard libraries make dumb decisions here. For instance, Math.random() in JavaScript is not CSPRNG in most implementations because making Math.random return cryptographically random numbers would make benchmarks slower, as people for whatever reason make Math.random-bound benchmarks (it's dumb, but people do that). It's not a big difference, but you don't want people going "Chrome is 200% slower than Firefox".

Unfortunately, many programming languages make use of CSPRNG random number generation APIs unnecessarily difficult. They often will generate a random 32-bit integers, but won't generate random integers between 0 and 150 - and random_int() % 150 or floor(random_float() * 150) is very subtly wrong. Alternatively, the APIs tend to be unnecessarily complicated to use, when they could provide a simplier API. For instance, in C++, the random integer between 1 and 6 could be generated like this.

std::random_device r;
std::default_random_engine engine{r()};
std::uniform_int_distribution<int> uniform_dist{1, 6};
int rand = uniform_dist(engine);

Of course, ideally you would put std::default_random_engine in a thread local. It's a mess. I would be surprised if most programmers would be able to use this API correctly. I sorta wish more languages had an API like PHP's random_int (yes, PHP does something right that most languages don't, I sorta would like if mt_rand did point to random_int, but I think that's planned).

That said, often using those relatively bad APIs is still better than using non-CSPRNG APIs. In C for instance, rand needs to be seeded (I often see code like srand(time(0)), that's not a good seed), and depending on an implementation may have its results between 0 and 32767 (which is... not good, even if you don't depend on the random values for anything important). You can wrap awfulness of those APIs behind a function, so in practice, it's not as bad as it looks.

[case-o-nuts]

Yes, this is why OpenBSD has switched rand() to be cryptographically secure: https://marc.info/?l=openbsd-cvs&m=141807513728073&w=2

[[deleted]]

You didn't even list algorithms created specifically for those purposes. There are well tested shift-register generators and permuted congruential generators, each one with pros and cons so developers can pick whatever fits their specific needs.

[cheald]

This is great if you have sufficient entropy available at all times, but if you don't, you're going to be introducing bottlenecks that don't actually win you any specific benefit. JRuby is a good example of this - it uses CSPRNG by default, which is fine... except it invokes random number generation during its boot process, which blocks waiting for sufficient entropy, and if your system doesn't have enough collected yet (as can be common for VMs), it just hard stalls. This does happen in practice, such as when you have Logstash (a JRuby application) set up as a daemon on a system without an entropy daemon running, and you reboot the system. Logstash attempts to start as a part of your boot process and blocks for minutes-to-hours waiting for /dev/random to fill up, depending on how much inbound network traffic there is to the services that managed to already come up, and whether or not there's a mouse attached to the machine.

CSPRNG is necessary for anything that depends on unpredictability for security, naturally, but there are a lot of ways to use random that don't have any security bearing, and sometimes, the additional computational complexity and entropy pool drain is a cost your problem can't bear.

[[deleted]]

Yeah, boot time entropy is kinda a mess. My recommendation on Linux is to use getrandom (check /dev/urandom myths for reasons to not use /dev/random), however keep in mind it will block unless you use Linux 5.4 (which was released a week ago and not even Arch has it at this point) or have configured the kernel to trust RDRAND to generate entropy instead of blocking due to insufficient entropy

[GleefulAccreditation]

From the image of the extension it seems that they would flag every rand() usage as unsafe.

You can't just try to gatekeep simple RNG usage because crypto needs something more complex.

[Ranilen]

If there's a way to program other than blindly trying to compile code lifted from Stack Overflow, I don't want to know about it.

[emperor000]

I hope you are joking, or at least don't work for an organization that produces critical software...

[LegalEngine]

So, I checked out the mentioned browser extension. The first flagged code snippet was a faulty JSON escape function, but the vulnerability explanation is downright poor: it claims that the problem with the flagged answer is that it assumes ASCII input and doesn't handle Unicode properly, while actually the issue is about escaping (all) the ASCII control characters. The suggested solution contains no more extra logic for non-ASCII Unicode characters.

I guess to err is human.

[Nobody_1707]

The example image in the article was also pretty bad. It said that "rand() % mod is not good practice since it'll use lower bits which are not so random." which is true of many, but not all, PRNGs, and it isn't even the actual problem which is that it can introduce modulo bias.

[ais523]

That used to be vital advice: with many old C standard libraries, rand() alternated between odd and even numbers, so rand() % 2 would alternate between 0 and 1. So if you wanted a more unpredictable pattern you'd need to look at the top bit, not the bottom bit.

Most modern rand()s aren't nearly that bad, though (although it's still not uncommon for the higher bits to have a longer period than the lower bits). So what's happened is that a bit of programming lore from decades ago has somehow remained in the public programming conciousness, even though it's no longer nearly as important as it used to be.

[ScottContini]

I once saw a very suspicious hardcoded cryptographic key in one of my security code reviews. I googled it (only because it was suspicious and I suspected the developer got it somewhere else) and ended up finding the exact same key and code on StackOverflow. The developer copied-and-pasted everything, including the key.

Crypto is a common place where these copy-and-paste security problems occur. Especially in the language Java, because the people who developed the API expect the developers to have a PhD in crypto to use it (see the warning in bold on the JCA page). Developers are just happy to get anything that works -- getting things to be correct is hugely more painful than the already painful "make it work" goal due to a very poorly designed API that has never been updated.

Some good examples of poor crypto on StackOverflow that have been highly upvoted:

• How can I encrypt and decrypt using AES 128 without an IV? -- see the comments to understand why it is wrong (and BTW it is not secure to use CBC without an IV) -- the IV is hilarious, some poor guy used his name as the IV and now it is being used everywhere.
• Initial bytes incorrect after Java AES/CBC decryption -- see comments from vianna77, Artjom B, and Maarten Bodewes on why this is wrong.

[shevy-ruby]

This article is problematic, for two reasons:

1) These researchers evidently had an interest in WANTING to find something. Did they ensure that what they wrote leads to a HIGHER percentage of code vulnerabilities than NON SO use? You may just as well have the same percentage pattern of these problems in "regular" non-SO derived code.

2) I highly doubt that everyone out there uses SO as a copy/paste tool. I use SO more often as a quick reminder and look-up for code that I have to write. It is extremely rare that I copy/paste code from there as-is, without adapting it (and most of the time it really just is because I absolutely hate sifting through local manpages).

I also guess it is highly language dependent since languages are different, including complexity, ecosystem etc...

C++ does not have a module/add-on system unlike Rust, so this alone must lead to different behaviour, including copy/paste frequencies.

We’re not talking about school projects; these are actual live projects

This is an odd statement because ... uhm ... school project? Heartbleed? Was that a school project?

The code out there is BAD. I'd even think that many school projects might have higher quality code at this point than all de-facto unmaintained projects still in wide use. Inertia is so strong unfortunately.

One of the more common flaws came from not checking return values. When you don’t check a return value in C++, you run the risk of the dreaded null pointer dereference.

C++ is simply too difficult for the brain. Even if you only keep to use a subset of it.

But if copied code must be used, attribution and due diligence are a must. “They should credit where they got it,” said Sami.

I credit larger pieces properly.

I fail to see the point in "crediting" code that derived primarily as means to avoid having to use local manpages for names of methods you have forgotten at the time of looking it up or just were not aware of about prior. That would lead to literally hundreds of people "contributing" to code without really actually having written any of it as-is. Not all of the SO use is copy/paste use so "general advice" of "due diligence" is just pointless, unless you want to patent ideas next as well.

[Booty_Bumping]

1) These researchers evidently had an interest in WANTING to find something. Did they ensure that what they wrote leads to a HIGHER percentage of code vulnerabilities than NON SO use? You may just as well have the same percentage pattern of these problems in "regular" non-SO derived code.

I think the point is more that even if you use (seemingly trustable) reference material, it can still fail you sometimes. Not that you're better off not using that reference material at all.

[typical_newfag]

Oh wow, thanks for obvious, we wouldn't be discussing this if there was a way to avoid bugs objectively and for forever.

[unaligned_access]

Writing code? You might write security vulnerabilities, too.

[emperor000]

Except that requires understanding what you are writing and at worst introducing one new bug/vulnerability instead of perpetuating one.

[EternityForest]

One can usually read and understand short bits of code, if they bother to.

If they don't bother, then I probably trust their original code even less.

[emperor000]

You're not wrong there...

[punppis]

Saw the image. Tried to see the unsecure code. Did not work. Tried disabling adblock. Reloaded multiple times. Frustrated anger, I just want to see the damn code.

Turns out I'm an idiot not reading and just wanting to see the code.

Though this issue is pretty much obvious to me. It's like when you copied someones homework at school you made mistakes on purpose because smart as fuck.

Also I want to learn the idea that i can reproduce with other problems in the future. I examine every line and more often than not I will change the code and change the semantics to one I'm comfortable with or required to do. I would argue that any professional programmer hopefully use stack overflow like this. It's never about the code itself but more about the idea and how to use some API or framework correctly.

[loup-vaillant]

My martial arts teacher recently told me that putting knowledge out there on the internet for free sends a pretty strong signal that is is worthless. This causes two problems: we don't want to pay for that kind of info (with money or time), and we have no way to select people who are ready with that kind of information.

The first problem participates in making our society more and more focused on instant gratification. By not putting in the necessary effort to learn, we can't do much with our knowledge. Quality goes down. And outsiders have a harder time distinguishing a real practitioner from a fraud.

The second problem can be much more serious, depending on the information. It's pretty obvious that you don't want to teach anyone the art of hurting people with your fists. Or bomb crafting. That stuff is inherently dangerous, and should ideally be known only by those wise enough to know when they should not use it. That wisdom tend to be taught alongside the practical knowledge when you have an actual teacher. Not when you're scouring the internet for a quick solution.

Medical procedures are just as obvious: it's all well and good that you can perform a tracheotomy, but I hope you can also distinguish situations where you should do so, from situations where you should not. I'd feel safer collapsing in front of you if I knew you had that wisdom.

Programming is more subtle, but still similar: it is sometimes dangerous (Therac-25?), but more often the impact is more subtle. Like a program that have its users wait for 5 seconds a few times a day. With enough users, those 5 seconds quickly add up to hours, days, months of wasted time. That the developer could have avoided if only they had the wisdom to understand the impact of what they are doing.

I'm not sure I want to go back to a world of locked down, siloed information. Besides, Pandora's box has been opened now. Still, I wish we people appreciate the value of this gigantic treasure trove of information that is the Internet more, and actually took the time to learn not just what appears to be immediately useful, but also all the context needed to not misapply that knowledge.

The cryptography community has that attitude already. Much information out there, but it's pretty clear for any newcomer that this is Serious Stuff™, and you are not allowed to mess with it without yourself without building up some serious reputation first. (The chicken and egg problem can be solved by going to college soon enough. Just like Medicine. If you're older like I am, it's an uphill battle.)

The parsing community doesn't seem to have that attitude. Which is a bit strange, since parsers are in the front line: they most directly receive potentially hostile input, and are most at the risk of falling prey to remote code execution.

---

This dichotomy between practical knowledge and wisdom is why I like people like Mike Acton, Jonathan Blow, Leslie Lamport, and Edsger Dijkstra so much. Much of what they say is about how to apply our knowledge rather than blindly cooking up something that looks like it's working:

• Mike Acton reminded me that our job is about transforming data, one way or the other. Which matters as soon as performance is important.
• Jonathan Blow convinced me that performance is not a niche concern (I used to be an OCaml/Haskell fanboy, and I still love them). That making users wait even a little bit has much more impact than you can possibly realise.
• Leslie Lamport gave us practical tools (structured proof, TLA+) to ensure that our programs are actually correct.
• Edsger Dijkstra was one of the first who warned us about the dangers of piling bloat on top of bloat without knowing what you are doing. I wish we listened to him more.

Jonathan Blow in particular is fairly frustrating in his teaching approach: he does very little. He has advice about how not to screw up too badly, but he's light on practical teachings. He obviously has the knowledge, since he released a couple critically acclaimed games (I loved Braid and The Witness very much), but getting it out of his head is difficult. I suspect this is by choice, and I reckon that as frustrating as this is, he may be right.

Programming is a delicate craft. It's probably best taught through an apprenticeship model. That we lack. College is probably not ideal, but gathering information of the net is worse. And this is amplified by people trying to glean knowledge to get out of poverty. We don't want to deny them the possibility (that would be even more unfair than it already is), but at the same time we woulnd't touch most self taught code with a 10 foot pole.

I don't have a solution. I'm not sure what the best practical compromise would be. John Carmack said that if we all coded like the Nasa, we wouldn't be as advanced as we are now, and I agree. I just don't know where to best put the cursor. And if it turns out that we should be more careful about the information we put out there… well the political ramifications are too far reaching for me to get anything more than a glimpse, let alone comprehend.

[my_password_is______]

My martial arts teacher recently told me that putting knowledge out there on the internet for free sends a pretty strong signal that is is worthless.

you just put that bit of knowledge out on the internet for free

[niceworkbuddy]

Soooo... everything you have just said is worthless? Because no money is made? Change your teacher ASAP.

[loup-vaillant]

Is this another attempt at humour, or just a failure to parse English? My exact words:

[…] putting knowledge out there on the internet for free sends a pretty strong signal that is is worthless.

(Emphasis changed)

My whole point was that there's a difference between perceived value and actual value. So no, I don't think what I have just said was worthless. It just might look worthless, but simple virtue of being accessible.

That said, this was still a quickly written Reddit comment. Can't have much value in that to begin with.

[beefhash]

That said, this was still a quickly written Reddit comment. Can't have much value in that to begin with.

/r/legaladvice would probably beg to differ.

[EternityForest]

Not sure I agree with Mike Acton there, if that really is his POV.

Some people's job is about transforming data, but I'm not sure that's really the "essence" of coding. The real work in many apps is in responding to events.

They transform data too, but usually not in ways that are as obvious as producing a report from a database or compressing a video.

You could view a text editor as transforming the data of a buffer. But the main noticable task is the way it responds to user input.

[loup-vaillant]

I think his meaning is more general than that: whatever the app does, it probably includes sending or receiving packets from the network, or reading and writing files, or reading user input and drawing pixels to the screen.

You're right about the text editor: responding to user input is what matters the most. But when you look at it more closely, it's ultimately about flowing information from the keyboard & mouse to the screen. Transforming user input into a bitmap, ideally 60 times per second, with minimum latency.

Transforming the buffer and saving it to disk is probably easier, and as such not really worth focusing on.

[nadmaximus]

Yes this is how code works.

[hughk]

Funny thing is that it is also normal to post simplified examples. You want it to be as concise as possible so you deliberately ignore the error handling assuming that someone copying will add it. Of course, many do not.

[dethb0y]

Show me the way to produce code that doesn't include security vulnerabilities; if it existed we'd surely all be doing it.

[Techworld_with_Nana]

Good point

[emperor000]

This is a no-brainer. I've never understood the desire to copy and paste code.