- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
i like these in particular
} // end for
} // end if hash_bytes
} // end if val non null
} // end if key non null
} // end while e.hasMoreElements
so THAT's what }'s do...!
Admin
"The function, getRandomBIts() returns a 32-yte array of random bites for security purposes."
BIts? ytes? bites? Make your mind up...
Still, good to see the CodeSOD back.
Edit by MC: Err, whoops. my B key doesn't work so well, and I missed that -_-;. Fixed now.
Admin
What an innovative way to do it! I bet he got paid by the line.
Admin
Makes you wonder how Random.nextBytes() is implemented.
Admin
This is definitely the hallmark of a good WTF...code that is twenty times more complex and introduces many new points of failure, while not being any more secure since it adds practically no entropy to the random data.
Admin
Admin
Although it doesn't make this any less of a WTF, I suspect the author was really trying to generate a GUID based on the fact that he was incorporating the system properties. The idea being to get a random sequence that was unique to the machine.
Admin
Wow! Knuth's going to have to add a new section to his chapter on random numbers.
Admin
It's been awhile since I touched Java, but at least on Win32, isn't there a direct API for generating GUIDs?
Admin
Oh, didn't you know? That's how the Java Random class is implemented internally anyway!
Admin
"Without further to do"?! Jesus. This site alone could be fodder for a linguistic WTF blog.
Admin
Well, Java is now open source, maybe I should look through the Random class code. Perpahs there is another WTF to be found there ;-).
Admin
Actually - I do some similar myself as it often helps my overview of the indentation, rather then having to trace an ending scope up to the starting point.
Admin
If this is in fact "for security purposes", then using Java's Random method would be as equally WTFy (and the five-line implementation is no more "correct" than the long one).
There is no "correct" way to generate pseudorandom data. That's a domain-dependent function; the PRNG has to meet the particular requirements of the task. In this case, of course, we have no information on those requirements other than "for security purposes", but if that's at all accurate we have some reason to believe that Java's Random would be quite wrong as well.
I rarely use Java, so my docs are well out of date, but I note that there at least used to be a java.security.SecureRandom, which at least claims to be suitable for some security purposes.
But what this item shows is that security code shouldn't be written by non-experts - and that's so well established it can hardly be counted a WTF.
Admin
Actually, the real WTF is both the "custom" random, and the suggestion to use the non-secure Random class for security purposes.
There is a standard class designed for secure random number generation, which is far more secure than MD4ing a bunch of somewhat predictable values.
http://java.sun.com/j2se/1.4.2/docs/api/java/security/SecureRandom.html
Admin
I don't think the author was going for a GUID. Yes, it's based on some values that are related to that particular system, but there's certainly no guarantee that the number is globally unique. You'd get just as good a GUID from plain old Random(). Face it, this is just a very poor implementation of a psuedo random number generator.
My favorite part is the bit where it eats the exception for grabbing system data. So... if something goes wrong, it will just silently hash the system clock? Awesome.
Admin
This is the trouble with java programming books. They talk all about sorts, arrays, etc. and so people think that you have to use sorts and arrays at all times. Someone should really come up with a book that is like a reference, and it just has 1000 examples, like, "to do random numbers, do this..." and then the shortest, cleanest possible way to do it. Maybe there'd be a chance that people would stop coding so much and get something done.
Admin
For a looooong block of code, sure. But for code you can see all on one page? Like for a 3 line for loop? You must be kidding.
Admin
"so THAT's what }'s do...!"
You'll notice that '}' is in fact an overloaded operator; sometimes it ends an 'if' block, sometimes it ends a 'for' block, sometimes a 'while' block... and that's just in this one code snippet!
It's entirely appropriate to add comments clarifying their usage.
Admin
If you are used to putting comments after }'s then you probably do it out of habit. I would say either do it or don't do it for the sake of consistency.
Admin
Really, those kinds of comments can be quite useful to the maintenance programmer. Maybe not here, but it's useful sometimes when you'll have logic that looks like this:
}
}
}
else
{
Having a comment lets you know which if that else corresponds to. Of course, there is usually some way to simplify the logic, but that's a different topic..
Admin
Tell me this was written by first-time co-op students. 'Cause that's what I was when I "invented" my own random number generator.
Admin
There's a mistake here: nextBytes() isn't a static method, so you should probably have something like:
I didn't know about the SecureRandom class--will remember that one.
Admin
I never said that this horrible code would actually give you a valid GUID, my suggestion was that perhaps he was trying to generate a GUID. For example, Microsoft's implementation for GUIDs is based on a system identifer and the current timestamp, which sounds an awful lot like what this guy was trying to do.
Admin
Almost makes me wonder if he copied someone's hashing code and gave it some pseudo-random data and called it "random".
Admin
Make it so....
Admin
I suppose this way is more "randommer" than the other way.
Admin
At least he's conciencous to his fellow threads while burning all those cycles.
Admin
Make that conscientious. Typical developer spelling skills ;-O
Admin
On the off chance anyone isn't aware, this is more or less how java's SecureRandom is implemented -- it fires up a bunch of threads, and uses data on how the OS allocates time to them as a source of entropy.
Admin
I realize this is a blog and not "The New Yorker", but talking about spelling, I would suggest the post writer would use some sort of automated spell-checker the next time. There were enough mistakes to make the post hard to understand.
Having said that, it seems to me that the function in question produces the randommest random that could be randommed. And, if the author was paid by code lines, he was smarter than we think.
Admin
Admin
I used to add these comments before the existence of modern* editors and IDEs that easily show the matching brace.
*Less than 10 years old
Admin
or... http://java.sun.com/developer/Books/effectivejava/
Admin
Not to start a flame war or anything..... :>..... but explaining which block the } ends is important if you do that lame-assed start-a-block-on-the-same-line-that-you-declare-it thing.
You know....
Admin
The real wtf here is that I have aids AMIRITE PPL???!
Admin
Aaugh! Please tell me that none of you laughing at this WTF will ever write cryptography code (excepting those few who pointed out that the built-in Random is bad too.)
This is the only way to produce a reasonably secure random number in software. Java.security.SecureRandom might be better, but guess how it works? By hashing together a bunch of system values, along with the time, a counter, recent mouse input, etc. And if the author of the snippet had some experience, he'd know to never trust the random functions that come with your library for crypto purposes, because they're usually crap. (The best thing to do would be to use java.security.SecureRandom, *and* all the stuff he's doing here, and hash those together.)
Admin
The documentation is qutie clear in the Javadocs how Random generates a number. If Random isn't appropriate, java also provides a few classes to randomly generate cryptographicaly secure keys. Either way, that Christmas Tree of code is quite the WTF? At least break it apart so it's readable.
Admin
Admin
I bet there was no requirement for the method to give identical numbers if called twice within a millisecond.
You'rre right about the SecureRandom part though, but using SecureRandom is just as simple as using Random. SecureRandom not just claims to be suitable for security purposes, you can choose from a number of standard algorithms (sure, there is no perfect one, but at least the built-in providers are definitely compliant with the relevant standards) and I think there should be native SecureRandom providers that use specialized random generator hardware.
Admin
% or M-x show-matching-paren
Admin
Admin
Maybe he was using the code itself as an input to another hashing algorithm?
Admin
Btw, isn't anyone annoyed by the magic numbers in this code? Being a maintainer, reading "(nbytes + 7)/8 " or "for( int i = 512/8;..." drives me crazy.
Admin
10 years? More like 30. Regular vi had code block navigation in 1976.
mathew
Admin
To reiterate what several posters have said because it's something that is poorly undestood - Random is a "psuedo random number generator". If you draw enough values the moments will converge to that of a uniform distribution, but they are highly predictable. The most basic implementation (linear congruential generator) is x(n) = a x(n-1) + p Mod M where x(n-1) is the last value in the sequence and x(n) is the next. a, p and M are constant parameters which have to be carefully selected. So if you know x(n-1) and the parameters - x(n) is known with 100% certainty! Random should only be used for Monte-Carlo simulation - not for Cryptography!
Admin
But some of the key points you are making is exactly what he is NOT doing, namely a counter (seed) and recent mouse input. His elaborate hashing of the System properties is pointless because they typically never change during the life of the VM's execution. Even his labor intensive hashing of the one randomish value he is mixing in, the system time, is pointless since he is looping a consant number of times, always shifting a constant amount, and always anding consant value, with no other seed or anything involved.
Admin
If you're going flame someone over his or her spelling, you could at least take the time to check your grammar. Perhaps there's some sort of automated grammar checker you can use?
That sentence is a comma splice, "spell-checker" should not be hyphenated, and in American English the comma goes inside that quotation marks.
See what happens when you go down this road?
Admin
You must have forgot the WITHOUT INDENTATION part...
Admin
SecureRandom is just a wrapper and delegates most of its work to an underlying "service provider" random generator. So if you don't like what the generators that Sun ships you can write your own and just plug it in.
You may not want to ask this guy though.