programming and human factors
by Jeff Atwood
In Why Isn't My Encryption.. Encrypting?, many were up in arms about the flawed function I posted. And rightfully so, as there was a huge mistake in that code that just about invalidates any so-called "encryption" it performs. But there's one small problem: I didn't write that function.
Now, I am certainly responsible for that function, in the sense that it magically appeared in our codebase one day -- and the the entire project is the sum of all the code contributed by every programmer working on it. I invoke the First Rule of Programming: It's Always Your Fault. And by "your", I don't mean the particular programmer who contributed this code, who shall remain blissfully nameless. I mean us -- the entire team. The onus is on us, as a team, to vet every line of code at the time it is contributed, and constantly peer review each other's code. It's a responsibility we shoulder together. Nobody owns the code, because everybody owns the code.
Yes, I failed. Because the team failed.
Geoff Weinhold left this prophetic comment on the post:
The irony in this is that someone will inevitably end up here for sample encryption code and blindly copy/paste your flawed code.
Indeed. Heaven forbid someone copy and paste flawed code from the internet into their project! In fact, a quick search on some of the unique strings in that original Encrypt() function turns up a few ... interesting ... search results.
That's just a sampling of the 131 web hits I got. To paraphrase Austin Powers, this Encrypt() function is like the village bicycle: everybody's had a ride. It's a shame this particular bicycle happens to have a crippling lack of brakes that makes it dangerous to ride, but what can you do.
Scott Hanselman coined a nice phrase for this: the internet as the bathroom wall of code.
It's true. People, being people, have gone and scrawled a bunch of random code graffiti all over the damn internet. Some of it is vanity tagging. Some of it is borderline vandalism. And some of it is art. How do we tell the difference?
That's the very reason I put forth a modest proposal for the copy and paste school of code reuse. Not that it would have helped in this case, but it sure would be nice if someone could perform a grep replace ...
s/Mode = CipherMode.ECB/Mode = CipherMode.CBC/g
... on, like, the entire internet. So other projects don't absorb this critically flawed code sample.
In the meantime, until that tool is developed, I recommend that you apply extra-strength peer review to any code snippets you absorb into your project from the bathroom wall of code. That internet code snippet you're looking at, the one that appears to be just what you're looking for, could also be random graffiti scrawled on a bathroom wall.
It's true that some bathrooms are nicer than others. But as we've learned, it pays to be especially careful when cribbing code from the internet.
"this Encrypt() function is like the village bicycle: everybody's had a ride. It's a shame this particular bicycle happens to have a crippling lack of brakes"
I would have gone with a herpes metaphor.
The 327th Male on May 19, 2009 11:26 AMStop it.
> s/Mode = CipherMode.ECB/Mode = CipherMode.CBC/g
It might offer marginally more protection, but cryptographic soundness comes from understanding the technology being employed.
The concept CBC > ECB is a dangerous one. Successful implementation of a cryptosystem requires understanding of all requirements of the application, and of the underlying tools.
From the previous threads it appears that most people who consider themselves knowledgeable about cryptography have fundamental flaws in understanding of it.
Errors in a CBC stream are isolated to just two blocks, the block effected by the error, and the block following that. Since the cipher text of following block is error free, error's do not propagate.
CBC mode also allows arbitrary injection of up to a block's worth of data. This possible attack would not be available in ECB mode.
Charles on May 19, 2009 11:30 AM"s/Mode = CipherMode.ECB/Mode = CipherMode.CBC/g"
Then there'd just be a heap of "encrypted" material in CBC with no (or implementation fixed) IV. Better sure, but hardly good.
hornetfig on May 19, 2009 11:31 AMYou could perhaps add some javascript call that catches mouse clicks, and then warns them that the code is unusable.
Anonymous on May 19, 2009 11:32 AMJeff Atwood might be stuck in Visual Studio land, but at least he knows how to write a substitute expression. Respect++
Paul Betts on May 19, 2009 11:37 AMCharles: indeed!
http://en.wikipedia.org/wiki/Block_cipher_modes_of_operation
Jeff:
Read on past OFB to the "Integrity protection and error propagation" section.
ECB, CBC, and OFB don't enforce the integrity of the message. Using one of the modes that does prevents attackers from manipulating the message.
Charles on May 19, 2009 11:57 AMAlso, I should have mentioned this last time, but the .NET calls create a random initialization vector for you already, if you don't specify one:
http://msdn.microsoft.com/en-us/library/09d0kyb3.aspx
"If the current Key property is null, the GenerateKey method is called to create a new random Key. If the current IV property is null, the GenerateIV method is called to create a new random IV."
which falls under the category of "provide sensible defaults".. although you need to pull out the IV and pass it back in to decrypt, in modes other than ECB.
Speaking of which, I just verified the TripleDESCryptoServiceProvider() picks CipherMode.CBC by default if you don't specify the Mode.
So part of the WTF of this Encrypt() snippet is overriding the defaults for no good reason.
Jeff Atwood on May 19, 2009 12:02 PM> Then there'd just be a heap of "encrypted" material in CBC with no (or implementation fixed) IV. Better sure, but hardly good.
Actually, the decryption (as copied from the intarwebs) throws an exception if you switch to the default CipherMode; you have to pass back in the randomly generated IV -- see above for MSDN ref.
Jeff Atwood on May 19, 2009 12:12 PMYou can always ask the "The Elders of the Internet" to apply your grep replace :P
Okay, that was a lame reference of the IT Crowd episode but hey, I love my bathroom full of gravity :D
Faj on May 19, 2009 12:20 PMIf only there was a website that developers could post things like code and others could vote on responses and comment on that code.
steve on May 19, 2009 12:26 PM"I recommend that you apply extra-strength peer review to any code snippets you absorb into your project from the bathroom wall of code"
It is very difficult to efficiently tag "bathroom wall" code, as contributors tend to hide embarassing things like copying code from the "bathroom wall"... you may detect some of the copy&paste, but most of the low quality contributions will be hidden by some minor changes or resegmentation of the code.
I would rather say: any mission-critical section of the code should undergo an extra-strength peer review.
And, if a project is using crypto for something more important than fun, the peer review should be conducted by someone who has some experience in the field, since the very nature of the encryption not only it is very easy to do mistakes, but they also are usually very good to hide themselves.
BTW, ECB mode HAS its fileds of practical application, but just not what was expected from who wrote the bathroom wall snippet.
Nabeshin on May 19, 2009 12:30 PM@steve
Ask Jeff, maybe he'll create one someday :)
katastrofa on May 19, 2009 12:32 PMAnyone thought perhaps Microsoft should just improve their documentation giving detailed descriptions of what modes to use, and include proper sample code with security implications outlined?
Flatliner DOA on May 19, 2009 12:37 PMIf you're going to use cryptography then please read Bruce Schneiers "Applied Cryptography" first.
It is not about choosing the strongest encryption algorithm or choosing the right cipher mode. It is all about designing a cryptographic protocol that solves your security problem. Encryption algorithms, cipher modes, hash function, digest, random numbers etc. are just lower level primitives used to build your protocols.
If you get into trouble then it's most probably not because of a weak encryption algorithm but because your protocol is insecure.
This also means that you can't just copy and paste a line of code or just change ECB to CBC. You actually have to understand what you are doing.
Heiko on May 20, 2009 2:00 AMDoes that bathroom graffiti pic say "For a lousy lay, call Betty Ann", followed by "For a lousy encryption algorithm, call Jeff Atwood" ?
Ian Johns on May 20, 2009 2:05 AMThere is a thing worse than posting bad code on your blog.
Posting bad code on your blog that someone else wrote.
But it can actually get worse.
Worse is not taking responsibility for posting bad code on your blog that someone else wrote.
What is worse than that? Not sure.
Andreas on May 20, 2009 2:06 AM"I recommend that you apply extra-strength peer review to any code snippets you absorb into your project from the bathroom wall of code"
... except you'd only copy and paste code from the web if you didn't know how to solve the problem yourself in the first place and, by Brian Kernighan's maxim:
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it."
So if you've got code from the net you're almost certainly not qualified to debug it - because you presumably weren't clever enough to write that code to start with by yourself (unless you temporarily forgot the exact API calls, for instance, but still knew exactly what you're doing).
The only solutions, IMHO, are:
1. Use the internet to understand the problem domain and then write your own code, knowing exactly what each line does
2. Use a third party library, plug-in etc. that is well respected in the community and knwon to be robust.
Jeff, why don't you start to apply the sed to your previous post ?
You don't think those "bathroom plagiarism idiots" are going to read both posts, do you ?
> 1. Use the internet to understand the problem domain and then write your own code, knowing exactly what each line does
I sort of touched on this in "when understanding means rewriting". It does force you to touch every line, and that's a de-facto code review..
http://www.codinghorror.com/blog/archives/000684.html
Jeff Atwood on May 20, 2009 2:17 AMRock on, TwitOfTheMonth. Best comment yet.
We Have a Winner on May 20, 2009 2:21 AM@steve
http://www.refactormycode.com/
I'd suggest putting a big, fat 'disclaimer' above your code, preferably in bright red saying "Every time you c/p this, a kitten gets gutted with a blunt machete. Also, your app will have a security hole the size of Mount Blanc."
Then again, people don't really care much for kittens any more these days.
n3rd on May 20, 2009 2:28 AMCBC has problems with openSSH: http://news.zdnet.com/2100-9595_22-303182.html
engtech on May 20, 2009 2:30 AMPop quiz: When did Jeff jump the shark? Was it the password screw up? The xss vulnerability? Or this latest encryption misconception?
Anon on May 20, 2009 2:48 AM> What is worse than that? Not sure.
Not learning from your mistakes is the worst of all, I'd say.
http://www.codinghorror.com/blog/archives/000300.html
---
"When I interviewed the surgeons who were fired, I used to leave the interview shaking," Bosk said. "I would hear these horrible stories about what they did wrong, but the thing was that they didn't know that what they did was wrong. In my interviewing, I began to develop what I thought was an indicator of whether someone was going to be a good surgeon or not. It was a couple of simple questions: Have you ever made a mistake? And, if so, what was your worst mistake? The people who said, 'Gee, I haven't really had one,' or, 'I've had a couple of bad outcomes but they were due to things outside my control' -- invariably those were the worst candidates. And the residents who said, 'I make mistakes all the time. There was this horrible thing that happened just yesterday and here's what it was.' They were the best. They had the ability to rethink everything that they'd done and imagine how they might have done it differently."
--
"@steve
Ask Jeff, maybe he'll create one someday :)"
In about six to eight weeks I guess!
Skizz
Skizz on May 20, 2009 2:52 AM"it sure would be nice if someone could perform a grep replace ..."
Actually it would not.
Jānis Veinbergs on May 20, 2009 3:13 AMSome people in thedailywtf.com claim that they've seen code copied from that website in code they've been working . . . irony, as thedailywtf.com is a wonderful repository of how to NOT do things in software development. :)
Thiago on May 20, 2009 3:31 AMYou'd actually *not* want the grep/sed (why is it called a grep?)
What you'd actually want is for the most critical bits of documentation to automatically appear in-your-face when reading whatever information.
This is a bit like wishing for drunk drivers to automatically get visuals of the injuries they might inflict and the grieving families etc.
Oh wait. I suppose this stuff is already there. Google 'ECB' or 'ECB +Encryption' and you'd have all the info you need.
What it boils down to: the problem is not the code being bad. The problem is copy/paster being bad.
> 5. Thou shall always use time-stamps and unique message counters to avoid replays.
Where a counter suggest incrementing numbers. From 20 years of IP stacks we know that this is not the most secure idea. Use random ID's
Seth on May 20, 2009 3:52 AMYou'd actually *not* want the grep/sed (why is it called a grep?)
What you'd actually want is for the most critical bits of documentation to automatically appear in-your-face when reading whatever information.
This is a bit like wishing for drunk drivers to automatically get visuals of the injuries they might inflict and the grieving families etc.
Oh wait. I suppose this stuff is already there. Google 'ECB' or 'ECB +Encryption' and you'd have all the info you need.
What it boils down to: the problem is not the code being bad. The problem is copy/paster being bad.
> 5. Thou shall always use time-stamps and unique message counters to avoid replays.
Where a counter suggest incrementing numbers. From 20 years of IP stacks we know that this is not the most secure idea. Use random ID's
Seth on May 20, 2009 3:52 AM> This is a bit like wishing for drunk drivers to automatically get visuals of the injuries they might inflict and the grieving families etc.
It might have been clearer to say it like this: "Thats much like wishing everybody grows a common sense overnight". It's just not gonna happen.
Responsible jobs require responsible people. You can't take the risk out of life without ending life. Things can only be simplified to a certain level and no further.
Attitude: deal with it, not "patch" or "mask" it
Seth on May 20, 2009 3:57 AMOkay, not *everybody* is responsible for every piece of code. When you actually promote that philosophy, what you really get is that nobody is responsible for any piece of code.
The responsible people are the author and the reviewer(s). And the author more than the reviewers, because reviewers usually expect that the author did something basically sensible, as the baseline for a review.
When I write a bug into the code, *I* did it, not the whole team.
-Max
Max Kanat-Alexander on May 20, 2009 4:22 AM>> 5. Thou shall always use time-stamps and unique message counters to avoid replays.
> Where a counter suggest incrementing numbers. From 20 years of IP stacks we know that this is not the most secure idea. Use random ID's
First, I think you mean TCP, not IP. Secondly, you would be using encryption and authentication, so hopefully you do not need to protect against the things the encryption-less TCP mostly unsucessfully has to try to protect against.
Thirdly, the point of it is to be able to know which data is outdated, so it has to be monotonically increasing. Which e.g. TCP sequence numbers are, even if they increase by random (small) amounts.
You don't need to peer review everything committed in the code base as long as you peer review every critical pieces of code (pretty much everything related to security).
As long as you test the features and they work, if it works why bother reviewing every single line of code (unless you don't test properly).
Mike Pool on May 20, 2009 4:30 AMWhen posting code, also post your unit tests. This way the programmer copying your code can see if your tests cover their scenarios. If not they can add to the test and make sure that your code works for them.
{6230289B-5BEE-409e-932A-2F01FA407A92}
Wayne Walter Berry on May 20, 2009 4:37 AM> This is one of the main reasons I prefer open source libraries to random code snippets. Open source code is tested and updated, nobody updates or maintains code snippets.
Yep that works: http://blogs.fsfe.org/tonnerre/?p=24
Blind trust in valgrind - the Debian OpenSSL vulnerability
Open source arguments are so boring...
Hi Jeff
Great piece as always!
Just some musing, this quote:
"I mean us -- the entire team. The onus is on us, as a team, to vet every line of code at the time it is contributed, and constantly peer review each other's code. It's a responsibility we shoulder together. Nobody owns the code, because everybody owns the code.
Yes, I failed. Because the team failed. "
Brillianty ilustrates why communist/socialist state could not prevail!
Everybody owns it, so nobody owns it; whereas each and everyone should put great care in nurturing it so that it could become ideal.
* for 'it', substitute 'the state' or 'the code; - see how it all makes sense?
;)
Have a great day!
MD
a dude form post-soviet country on May 20, 2009 5:07 AM> Blind trust in valgrind - the Debian OpenSSL vulnerability
I think Jon's point is that code absorbed from a living open source project is a much nicer bathroom. A random code snippet from a random website is certainly a dive bar sort of bathroom..
Jeff Atwood on May 20, 2009 5:08 AMYou suggest code reviews to catch these kind of problems, but I'm willing to but 99.9% of code reviewers would not catch this issue (assuming the code reviewer doesn't follow this blog).
fschwiet on May 20, 2009 5:54 AM(earlier posted this comment to wrong post)
I am surprised you didn't find this example:
http://support.microsoft.com/kb/317535
huh?
Charles on May 20, 2009 6:08 AM@All of the Ivory Tower crew, as TwitOfTheMonth named them:
If you are into flaming, my opinion is that comments are not the correct place.
However if you still need to fullfil your thirst for arguments, may be the Monty Python can answer you, as flaming is not welcomed here:
http://www.davidpbrown.co.uk/jokes/monty-python-arguement.html (no offense meant, please read this with sufficient humor).
It's true, Meredith DOES have a fetish for 18th century dandy fops!
Tech Introvert on May 20, 2009 6:19 AMThere is a perfect solution to get rid of this bathroom wall of code. First create a question and answer website. Then, people will have to vote on the answer to make sure that only the correct solution gets flagged...
J on May 20, 2009 6:28 AM
@Seth
"Why is it called a GREP"
Comes from the really early UNIX days. Get Regular Expression Print.
Matt on May 20, 2009 6:31 AM"The onus is on us, as a team, to vet every line of code at the time it is contributed, and constantly peer review each other's code"
Ideally that would be the situation. Peer review everything.
What do you do when you are in a situation where you are working on a project that, when asked at the beginning, you estimated would take 9 months and management tells you that you only have 5 months starting 2 weeks ago? And in addition to the time constraint, the clients are scrutinizing every little cost and are just waiting to freak out all over the place.
Oh well, I guess it's the old hurry up and code because you are going to need a lot of time to debug...
Ryan on May 20, 2009 6:51 AM> Blind trust in valgrind - the Debian OpenSSL vulnerability
>I think Jon's point is that code absorbed from a living open source project is a much nicer bathroom. A random code snippet from a random website is certainly a dive bar sort of bathroom..
...and if you cut 'n' pasted the code it is now yours to maintain and fix, whereas the Debian OpenSSL vulnerability was fixed, as a library, soon after the problem was discovered, recompile with the new library and it's fixed...and you can be as sure as far as is possible that a lot of people have checked it better than you could ...
Jaster on May 20, 2009 7:16 AMSurely if you ran that regular expression on the whole internet then this article's regex would look like:
s/Mode = CipherMode.CBC/Mode = CipherMode.CBC/g
Off subject:
Riding brakeless is the biggest trend in bmx nowadays. I'm sure the shoe companies love it.
Joe Beam on May 20, 2009 7:39 AM"The onus is on us..."
This is an interesting phrase.
Bryan Roth on May 20, 2009 8:07 AMOh Noes. Someone wrote something on the internet
... and it's wrong!!
Seriously, if someone copies that code without RTFA, they deserve whatever they get. If they need to be protected from that, they should not be programming, and should live in a nerf universe.
Kudos Jeff for having the cajones to admit that you fkd up, and doing so in such a public way. It does happen to human beings, and programmers (open source included) too.
To paraphrase and mangle together a few different quotes:
Good judgment comes from experience. Experience comes from bad judgment.
The person who never made a mistake never learned a thing.
Just try to suck less every day.
If you want to prevent dumb copypasta, at least change your code snippet to an image so it can't be copied.
BBT on May 20, 2009 8:17 AMChanging it to an image won't prevent people from using it, it will just take them longer. Use no code from the internet without first reviewing and understanding it.
@Tech Introvert: Yes she does!
Bratch on May 20, 2009 8:35 AMI added automatic commenting functionality to Snip-It Pro, a code organizer program I created to manage code snippets after Jeff's last post, the modest proposal for copy paste code reuse. This way at least with the snippets you and your team use, you'll have some way of tracking (either by guid or reference url).
I released the new verson a few days ago.
http://www.snipitpro.com/
@Jeff,
Couldn't you use StackOverflow's software to create www.BathroomWallOfCode.com, and then a user could add their piece of code and others could comment on it or wikiEdit it or vote it up/down. You could tag it C# Encryption and then people who know things about encryption could poke holes in this code. Perhaps then you could have notifications when the code has been updated.
What do you think?
You don't have enough on the go right now do you? ;-)
Nathan on May 20, 2009 9:01 AMPeer review is one thing, testing your own damn code is another!
What's the point in coding anything if you don't check it actually works?
Dave on May 20, 2009 9:04 AM> As long as you test the features and they work, if it works why bother reviewing every single line of code (unless you don't test properly).
Um, because it will need to be maintained. To make sure that while it may "work", it doesn't do so in the worst possible way. Mutual growth. ...
mt on May 20, 2009 9:13 AMI think the whole point is being missed here. Two-way hashing (encryption / decryption) is only valid in scenarios where you need two way communication (for instance data in transit). Authentication is purely one way. Hence the more secure method of storing an authentication token is one-way hashing http://en.wikipedia.org/wiki/One-way_hash with a salt (for instance data at rest which can be recreated).
Consider the several use cases:
1) User creates password (this use case they will never, ever need to see their created password again). The value that is stored in the database is the one-way hash
2) User needs to log in (this again never states that the user or the system needs to know the password, just the generated one-way hash and whether the hash matches or not). The user types in the password to authenticate, which is convertted to the one-way hash and compared to the hash value in the database.
3) User needs to reset their password (this final scenario still does not require the system or the user to decrypt the password hash, you just make them go through what ever hoops needed to reset the password, and you overwrite the hash). The user types in a new password which is hashed + salted and put in the database in that format.
The key here is that the hash is built in such a way that it cannot be decrypted. That is by design. Anything data that you store at rest, that you consider any level of restricted access, that can be recreated and compared, should be stored in a one-way hash.
Also, as Jon Galloway stated, the bad code is still floating around on the internet. Since you cannot change the internet, you should change how you view any code you see on the internet. I personally view all code I find on the internet as a good foundation to build my design around, but beyond that is subject to much scrutiny.
You make very valid points about the code reviewing process. I am a very strong proponent of that on a development team. In my mind, snippets should only be used if the developer who used it can provide evidence that they can recreate the snippet on paper and away from the internet (maybe a bit harsh, but yes that is what I believe).
(Sorry if this post sounded critical, that was not it's intent. Let's face it though, you will always sound critical on things that you are passionate about and therefore criticism should be expected).
Justin Wendlandt on May 20, 2009 9:15 AMForget peer review; try some TDD. Then if your pasted method didn't work, you would've known it.
Also, if you publish code on the Internet, publish the unit tests alongside it.
Kevin Wong on May 20, 2009 9:59 AM@Kevin, how would TDD have solved this issue?
Nathan on May 20, 2009 10:19 AM@Clinton,
Perhaps... but the blog post name was "Why isn't my encryption... encrypting", that I believe is a big enough hint to not copy the code. There is still the responsibility on the person copying/pasting the code.
Nathan on May 20, 2009 10:21 AM"Forget peer review; try some TDD. Then if your pasted method didn't work, you would've known it."
Kevin, how can you write a test that captures the knowledge of a cryptography expert? How is a unit test going to know which ciphers are trivial to crack and which are state of the art? You need cryptography experts to review your design choices.
"repeating the broken code"
Clinton, if someone is stupid enough to copy and paste without reading the entire article then they're too dumb to get security right anyhow. You can't copy paste expertise, despite how often code monkeys try. :(
You plagiarized and then realized (thanks to your wonderful readers) that the function (which you supposedly wrote) is not working as it should.
And now you are embarrassed, because you of all people should have been able to figure it out.
Hence you wrote this piece of (shit) blog post trying to convince your readers otherwise, trashing your goddamn philosophy upon us.
You are just covering up man.
Own up, take responsibility.
And you should have never plagiarized in the first place.
I am so disgusted by this (you of all people should be a responsible mature programmer) that I tore my ticket to your StackOverflow Dev Days - that's $10 I am not seeing again.
This sounds like a great feature for Stack Overflow: a place to post helpful code snippets (or perhaps this is already evolving now?). There are many code snippet repositories out there now, but the Stack Overflow model would have the added benefit of:
1) Bubbling the best snippets for a particular issue to the top.
2) Being a wiki, so that others can refine and improve snippets.
monsur on May 20, 2009 11:25 AM+1 for including snippets of incorrect code as images...maybe with a big water-mark that says "[FAIL]" or similar.
Copying code without understanding or testing it is a symptom of laziness or time-pressure. Lazy or time-poor people will most likely just search for dodgy code elsewhere rather than transcribe YOUR dodgy code.
JosephCooney on May 20, 2009 11:26 AMWhat library would the crowd suggest for a .Net developer who prefers to let the experts do what they do?
EntLib?
Bouncy Castle?
> "Why is it called a GREP"
> Comes from the really early UNIX days. Get Regular Expression Print.
Nope. That's just an Urban Legend.
Ken Thompson created the grep command at the request of Douglas McIlroy who needed a way to examine multiple dictionary files for a speech synthesis program he was working on back in 1973.
McIlroy was examining multiple dictionary files and was using the old "ed" Unix editor. The "ed" editor was not a visual editor. Instead, there was an internal pointer that pointed to a line (but didn't necessarily display the line). You could add lines, delete lines, or use the substitute command to edit the lines.
One command in ed was the "g" or "global" command. It allowed you to operate on all lines in the file at once. Another command was the "p" or "print" command that printed the lines. If you combined them as "g/re/p", you would find all lines that contained the regular expressions "re" and print them out.
The problem is that "ed" operated on one file at a time, and that was slowing down McIlroy's work since he had to examine multiple files. Thus, Thompson took the parts of the "ed" code he needed, allowed it to operate on multiple files at once, and called it "grep" after "g/re/p".
David W. on May 20, 2009 12:12 PMI'm glad Jeff finally addressed the 'use an open source library' point, although I fear it won't really sink in. Jeff, this is the second high-profile occurrence of you screwing up code and creating security holes because a) you don't really know what you're doing and b) you're trying to do it yourself.
I get the impression that your reluctance to use high-quality, open, peer-reviewed code stems from your addiction to Windows programming. I can hardly blame you for having to write code yourself - Windows just doesn't offer a powerful development environment, so open-source has hardly thrived 'there' - but you might want to check out some of the free alternatives if you want to prevent a repeat of this embarrassment.
Brian on May 20, 2009 12:16 PM@Nathan and Derrick
Oh, I don't know. You could run these and look for repeated blocks in the output:
Encrypt("try some different" +
"00000000000000000000000000000000",
key, true).Base64ToHex();
Encrypt("salts" +
"00000000000000000000000000000000",
key, true).Base64ToHex();
My point was, though, that if you're copying code from the Internet, try to get the unit tests with it. They'll tell you what it does, and you can compare that with your requirements. Otherwise, write the tests yourself. (Of course, cryptography requirements might be more difficult to quantify and codify in tests than most.)
Kevin Wong on May 20, 2009 12:24 PM@Matt
grep is "_Global_ regular expression print". The original grep was a specialization of the (pre-Unix, teletype era) editor "ed", where "/foo/p" prints the *next* line matching the regular expression "/foo/", but "g/foo/p" prints *all* lines matching the regular expression "/foo/". See "Software Tools" by Kernighan and Plauger.
I have a completely unsubstantiated hunch that it was also a pun on "grope" - as in "groping in the dark".
@steve & everybody else asking Jeff to make websites..
Please, stop that. If he listens to you and makes another site, then we won't get blog posts for like months at a time. Just stop it. :)
If only we could find some way to legitimately nest the offending code in tags. :)
Hutch on May 20, 2009 12:54 PMToo steal a cheesy quote, there is a difference between knowing the path and walking the path. Jeff doesn't claim to be an expert, and sometimes non-experts make mistakes (who am I kidding, experts screw up too, just like all those people who thought MD4 was good enough, and those that slobbered over MD5, and some day we'll mock 3DES and whatever replaces it).
@Clinton Pierce:
Grow up. Take responsibility for your own reading. If you don't read the whole article, who can you blame?
@Brian:
Jeff uses the tools he uses. Pointing out that I could own a Chevy Corvette isn't helpful when I can only afford a Geo Metro. Unless you're offering to cover the cost difference. Until then the rest of us use the tools we can to do what we can.
@All of the Ivory Tower crew:
Get a grip. Most of us have deadlines and we make the choices we make because of the situation we're in. Frequently we know there is a better way to do it, but can't expend the resources required to develop or integrate that solution. There is a difference between a Computer Scientist and a Computer Programmer, apparently one that you've overlooked. Jeff isn't a Ph.D. busily whipping his grad students to publish his rehashed theories. He has a job and family and he (theoretically) does the best he can. If you'd like to have a protracted debate over the degree to which I ought to normalize a database or how much performance I can squeeze out by in-lining a function, that's fine, but I get paid to produce, not to theorize. Stop bustin' Jeff's chops because you'd do thing differently because, in fact, you AREN'T doing it.
Sheesh.
TwitOfTheMonth on May 20, 2009 12:58 PMSo we might not have grep-the-internet commands yet, but we do have services like sharedcopy and friends, which teach us that comments should not be tied to the page, they should be free! I can edit the page with my sharedcopy edit button, and other users can see those changes, so the only question at hand is how to standardize such things to make them more accessible.
Calvin Spealman on May 21, 2009 5:10 AM@MW: "Blind trust in valgrind - the Debian OpenSSL vulnerability"
Blind trust in unowned / uninitialized memory, a generic OpenSSL vulnerability. A secure system would clear stack memory on return, destroying entropy that it's trying to fold in. Debian screwed up, but OpenSSL does need to fix it.
Rob on May 21, 2009 11:16 AMHear, hear. Somebody (who shall remain nameless) on our team managed to introduce some 3ncrypt10n code into our software, with the private key as a constant string IN the source code (strings.exe, anyone?).
This person was above peer review for political reasons, but I managed to find this unacceptable nugget in the subversion log one day and fix the problem.
Null on May 21, 2009 11:47 AM@everyone missing my point about grep
Thansk for some interesting historical tales.
However, as far as I know, there is version/variant of grep that does replaces. Go figure
Seth on May 22, 2009 3:02 AMSheesh. I should proofread my comments -
s/Thansk/Thanks/
s/there is version/there is no version/
“I recommend that you apply extra-strength peer review to any code snippets you absorb into your project from the bathroom wall of code.”
Surely the recommendation should be to not copy and paste from the internet. (I thought that was the point of your modest proposal). There’s no guarantee that any amount of peer reviews would find a problem like this, because by definition the peers are going to be as clueless as the one who had to crib from the internet. If it’s a security issue it really needs to be reviewed by a security expert, and if you are copying random snippets I would have thought a lawyer should be checking it.
(what, no captcha?)
Steve W on May 23, 2009 3:30 AMFor dumb copy&paste i think it is enought for a heading something like "Bad code:"
Who in there right mind would copy a piece of encryption code that was filed under the header 'Why Isn't My Encryption.. Encrypting?'
Qua on May 24, 2009 5:45 AMWorth note, a wrong piece of code is more likely to be mentioned and quoted around the interwebs, precisely __because__ it's wrong. So I'd guess popular code snipets on google have in average more chances of being wrong than those on page two.
(I only skimmed the comments, so forgive me if I'm being redundant).
Bruno on May 25, 2009 11:52 AM> The irony in this is that someone will inevitably end up here for
> sample encryption code and blindly copy/paste your flawed code.
Copy/paste without using your brain is definetly not the good thing to do, but I strongly believe than it is very instructive to read other's mistake.
This is why I've designed a mini website to show the most common programming error: http://www.badprogramming.com.
I'd guess popular code snipets on google have in average more chances of being wrong than those on page two.
links of london on August 26, 2009 1:20 PMI'd guess popular code snipets on google have in average more chances of being wrong than those on page two.
abercrombie and fitch on August 27, 2009 12:54 PMWhy i can't post it. http://www.christianlouboutins.de
christian louboutin on August 30, 2009 9:21 AMI think the real problem with your bad code is not that people will cut and paste code from the internet without trying to understand it. It's that they will waste time trying to understand something that's broken, and perhaps drawing incorrect conclusions in the meantime.
m4bwav on September 30, 2009 3:19 AMGreat post and draw. Thank you for sharing.
wow power leveling on October 21, 2009 7:06 AMSaw a snippet from http://gist.github.com and immediate thought of this post...would be nice if they allowed comments too.
lmsurprenant on October 22, 2009 8:48 AMThanks for your information, i have read it, very good！
ed hardy clothing on October 22, 2009 9:22 AMThanks for your information, i have read it, very good！
ed hardy shirts on October 23, 2009 8:11 AMgreat info and really useful things on the website
high risk merchant account on October 23, 2009 12:20 PMThanks for your information, i have read it, very good！
Very cool! Congrats on the pairing.
street lamps on October 24, 2009 2:50 AMYour thinks are very good, I will do like you say,The bathroom wall I will do like it.
valves manufacturer on October 26, 2009 6:59 AMVery good, thanks very much.
fittings on October 27, 2009 6:56 AMI like your article.
Coagulant on October 27, 2009 7:44 AMGood opinion~~
I'll recoment your blogs.
Ahha, I like your blogs.
tungsten carbide on October 27, 2009 7:48 AMThis blog page is the best I had seen,very good.
butterfly valves on October 28, 2009 10:02 AMThe comments to this entry are closed.
Subscribe in a reader
Subscribe via email
|
|
Traffic Stats |