- 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
Admin
Admin
Which would actually be a WTF, since short tags have apparently been depreciated to avoid language conflicts.
I've not studied it too long, but as for the original code, this is an interesting point....
The commenting: serious wtf The exact calls: interesting ... but only slightly broken ... the cast should simply discard everything after the the first non-numeric character and return the first part (atoi style) and in this case that is the ms component, which is a good random seed candidate. The concept: spot on. To me this looks like a text book (if a little long winded) method of generating a random string of characters conforming to a predictable character set.
The only thing questionable is the logic for using it, but if someone loads pages fast, they could experience caching issues, so this method would, technically speaking, be more reliable.
To other commenters...
Ah, if only this were true. As has been pointed out multiple times, not all browsers pay attention to this, and worse, ISP level proxies don't always pay attention to the caching headers. Don't ya love people that don't follow standards?
Again, as pointed out, the purpose of salt is to prevent someone using a rainbow table to break a hash.
And in this case, I think the use does conform to the definition, since it is a string being used to influence the transformation process between the source and digest.
Yes well, dynamic content and ISP level caches... do I need to elaborate further?
And now for the fun one ...
Ok, so "source pool" or "secret" or something might have been better, but as I said, I think salt is close enough to being valid in this case. Ah, isn't ignorance bliss? Unless I'm much mistaken, pre 4.2, s/rand() behaved exactly like C's s/rand() ... meaning that every start up, the seed was set to 0, and the same sequence of numbers is generated. In any case, calling srand() is both sensible and properly backward compatible. Wait, what's a wtf in this? You have a better way of turning a string 0 <= x < 1 in to an int than, atoi-esq cast, multiply, implicit cast? And for anyone wondering, this is technically less predictable and more reliable for this purpose than seeding from time(), since two calls in the same second would get the same random string...Wait .... To simplify the process here... You have a number between 0 and 999,999 ... if you do 999,999%33, you get 0... 30,303 * 33 == 999,999... so that means that you have an extra 0? How big is that skew? 0.003%?
A truly random number source has a bigger skew than that.
The irony is, of course, that in the very next breath you mention the fact that we're not using mod 36 here, when that would introduce a far bigger skew.
Care to suggest a better way? Assign to an array and join it maybe? Have 7 temp vars knocking around and concat them in one go?Done much C lately?
One would assume that it is there for the purpose of demonstration?That was a nice rant.
I'm sure this won't get featured since it's waaaay too long, but hey, that's life.
And for the record, The Real WTF is the comments we generate here... and the number that open their mouths without thinking first. Occam's Razor may prompt you to assume incompetence in something you don't understand, but never forget there might be a good reason for something strange.
Admin
Ah, I get it - TRWTF is that he forgot to do the dictionary lookups to make sure there weren't any rude words in the random string. Maybe that's why they call it "salty language"..
Admin
The comments! They burn!
Admin
// set i = 0 $i = 0;
Gee thanks so much!
Admin
WTF. your code is not going to be as tasty without the salt. You should consider using less pepper (comments) though.
Admin
Been there, did that. I had a cow orker that would argue any change from the status quo until I got tired (he didn't debate, just brought up the same argument and tried to talk me to death), so when I found brain damage in the code, I could either argue for a month and get nothing done, or just fix it and have it go in.
Another irritating habit: discuss some change, get him to agree to do it a certain way, and have him go back and do it his way anyway.
Admin
The real wtf is that instead of using rand they should have used mt_rand. Duh!
Admin
Favorite comment of all time:
2nd favorite:
Great!
(couldn't leave it open)
Admin
| Why seed the randomizer with (double)microtime()*1000000, | when microtime() already returns a string that looks | like '0.53138500 1203062920'? Why srand - which is | basically deprecated as of PHP4, and accepts an int
Because PHP will evaluate it like this:
'0.53138500 1203062920' * 1000000 => 0.531385 * 1000000 => 531385
This part actually looks ok to me.
Admin
Hi Nazca! Thanks for the analysis.
First, I'd like to say that not all of my comments were intended to mock. I really was trying to represent the process of thought that might have produced that code, and there were some good things in there.
But this one is always the same string, which makes it not a salt. The whole point of a salt is that it's different for every use. Thanks, I try :) $source could work. $secret could not, because everyone knows the alphabet, and nothing in this function relies on it being secret. $salt is wrong because it has a definite meaning and this is not it, so using that name will mislead readers. Personally I would have gone for $alphabet. Not in the sense of our alphabet a-z, but in the sense of being the set of symbols for this algorithm. Well, in theory. In practice I would just use $s because it's short :) Yeah, but calling it before every use is not. srand() should be called once, at startup. Having srand() inside the function will actually degrade randomness if you call the function twice. No wtf in the code. This was my stealth way of answering some of the questions of the submitter, which had a wtf factor of their own :) The skew is small, but it just annoys me when people don't get it right. It's one of my pet peeves, especially since I play online strategy games :) And it can have a big effect when you're working with individual random bytes, so it's a pattern to be suspicious of.The PHP docs say that RAND_MAX might be as low as 32768. But to my surprise, assuming they mean 0 to 32768 inclusive (which is a bit weird), it turns out that that particular range doesn't have any skew when taken modulo 33! Maybe this explains why the code doesn't use 36 :)
Oh I don't exactly disagree with this method, I just couldn't resist the pun :) When you're building a string this short it probably doesn't matter what method you use. But PHP does allow something like this, similar to how you'd do it in C: That's why good commenting is so important, though! For example, there should have been a comment somewhere that explained why the NoCache parameter was needed at all, and which browser specifically caused the problems. That allows future developers to test new pages against that browser, and check if it's still relevant. Not having such comments means that new developers have to keep copying the old code without knowing why, which leads to cargo cult programming.I've seen my share of /* Do not delete the following line!!! */ followed by a nonsensical line, and surrounded by a page full of hacks to keep the rest of the code working in the presence of that nonsensical line. Presumably the line made some sort of sense, eight years ago.
Admin
The phrase "previous developers" indicates to me that there may not be anyone around to ask why they did so. Just to give them the benefit of the doubt.
Admin
Actually
would be a Brillant! and enterprisey solution guaranteed to work in all of our top-quality codebases!Admin
For the folks calling this dubious, I'd agree on some level; it's not a real WTF, but more of a noob issue (i.e., not understanding what is built in to the language). The whole thing could have been done more succinctly, albeit a little more verbose than the example below (the ASCII table has some non-URL-safe characters between 48 and 122, so there would need to be a check for that):
Far from perfect, but the point is the only WTF is either not being seasoned enough to know what the lang is capable of (something that only comes with time, so not really a WTF) or failing to find a better way of doing something that is, at a theoretical level, perfectly valid (something that can only be chalked up to laziness, which is the worst kind of WTF).
captcha = jumentum (or "momentum as it applies to the Hebrew population")
Admin
I've had the displeasure of working on code commented like this code. I honestly don't understand what motivates people to put in comments like
// set i to 0 $i = 0;
Are you just commenting those lines so that you can say that you comment your code? Why don't you comment the reasoning behind your bat-shit insane algorithm instead of giving me a lecture on how variable assignment works in the particular language that the code is written in?
Admin
Look at the big brain on Brad!!!
Admin
The Real WTF is use of PHP
Admin
Lets say I have a password database and one of my users has the password "Dancer365" ... if all the passwords use the same salt that still means the attacker has to generate an entire rainbow table to get a match (or collision). Using the same salt does defeat part of the point, but not the whole point.
hehe :) Everyone knows the alphabet, but not everyone knows that $secret contains that. It could be really well shuffled. Trivial to guess, but still secret. Point about it not needing to be secret, though I still defend the validity of using the term salt for this. For the rest, I agree.mmm, not really... Calling it before each use makes sure that it has actually been called rather than simply assuming the right header got included (from the tone of the OP, I'd say that's probably not a safe assumption) Degrading randomness... any instruction consumes time ... the chances of getting the same seed again is 1,000,000:1 ... I haven't actually tested the effect on randomness by repeatedly seeding the prng, but I would guess the numbers would be less predictable if nothing else.
Ah, fair enough :)Ah dear ... looking at what you quoted me as saying, I clearly wasn't paying attention and stuffed it up lol. I actually did math for someone performing modulo of the seed value, which is simply stupid. I'll take your work for 32767%33.
Of course we could be missing the simple answer that somewhere in the process of typing the OP, someone hit 3 instead of 6 on the numpad :p
hehe, yes, that would work at that ... though it's not as clear as the string concat ... though ... isn't it $s{$i}? meh, can't really remember. Yes, cargo cult programming is alllllways fun hehehe. Definitely agreed, though the problem is that the software it would have problems with is likely many and obscure :(Admin
I startled my coworkers with my fit of audible mirth on reading your post. This part in particular, is obviously a reference to the old joke:
Why is 6 scared?
Luvverly captcha for Latin scholars like me: consequat
Admin
// Ask WTF Anthony was thinking Is this WTF meant to be the code or, the solution Anthony C came up with?
// Really what was he thinking I fixed the symptom not the problem it's self.
Admin
Firefox has an awful caching system.
http://particletree.com/notebook/firefox-caching-issues/
I'm generating dynamic images through php (charts/graphs) and firefox refuses to acknowledge the http headers.
Random request strings like these are the only way to get it to behave.
Admin
... the bug being "Not enough bad style and useless code".
Admin
cos seven ate nine's mum
Admin
This are the best comments I've never seen in a piece of c.... code ;-)
Admin
no caching? maybe this is a solutiion:
<meta http-equiv="Cache-Control" content="no-store, no-cache, must-revalidate, post-check=0, pre-check=0"> <meta http-equiv="Pragma" content="no-cache">Admin
I love these incredibly useful comments in the code:
Admin
Haha, the poster of this also made a WTF! He should have used the shortcut, <?=time()?> Better yet, he should have dropped that from the url entirely, and instead make sure the referred php script sent the proper no-cache headers.
Admin
http://en.wikipedia.org/wiki/Salt_(cryptography)
Admin
PEPPER is generally usefull to keep people from getting too nosy ;-)
Da' Man
Admin
Admin
Guys string is 8 character string, because it's from 0 to <=7 including, Making all your sarcasm a BS
Admin
Addendum (2008-05-13 06:10):
Admin
Anthony C said, "I decided against [...] even trying to figure out why my predecessors chose such an elaborate routine to generate a random string to prevent a browser from cacheing."
Is Anthony one of those people who thinks that "cache" is pronounced in the same way as "cachet"? Maybe he meant to write "cachéing" :)
Admin
A salt is used to protect against dictionary attacks.
(From Google): "A seed value used in the encryption of a plaintext password to expand the number of possible resulting ciphertexts from a given plaintext. The use of a salt value is a defensive measure used to protect encrypted passwords against dictionary attacks."
Incorrect use in that context though.
Admin
Admin
Well, a "salt" could be a prefix string when using hashing. For example, in the unix passwd file, the hash values for user passwords are in fact not just directly the hash of the passwords, but the passwords prepended with salt (ok, may not work it exactly this way, but this is the idea).
So a salt could be a "randomizer string" in this sense.
Admin
// Set i = 0 $i = 0;
What I especially love about this wtf is how if you change the assignment you must update the comment.
// Set i = 0 $i = 1;
OOPS THE COMMENT MAY CAUSE A BUG NOW.
Admin
Yes, gotta love the people that continue to post the same stupid answers without reading the comments and seeing that they've already been talked to death.
Admin
Salt is an old-ish term. I think it was mostly used with the crypt(...) system call in *nix systems.
http://en.wikipedia.org/wiki/Crypt_(Unix)
Admin
So uh... what happens if the request is made more than one time in a second with the proposed fix?
Admin
Why not simply use rand() instead of all the hickups mentioned here?
The pattern of adding something random to a query-string makes perfect sense to me. Using time()-also makes sense. Perhaps even md5(rand()) would make it for a nice string :-)
Admin
Use of time() to prevent caching is a WTF on its own. There's Cache-control:no-cache for this purpose and it doesn't pollute caches with never-reused copies. I'm browsing on a mobile device with limited cache capacity you insensitive clod!
Admin
Its also not truly a random value that its generating. 33 doesn't evenly divide 1000000, so 'a' is going to be a more common character than the others. Which is a bit scary, as the term 'salt' implies the original developer thought they were doing something involving cryptography.
Admin
Yes, if the same user requested the same page twice at the same time from the same browser session, then yes, the time method would "fail".
So in reality, it's ok.
Admin
That's from the old school of structured programming. Where you actually write down what you want to do in human language before you try to code it. I'd rather have lots of silly comment than no comments at all.
You can say "Don't comment your code, code your comments." But no one does.
Admin
The real WTF is that you don't understand at all HTTP. Add cache control rules to the HTTP output and drop that bloody stupid & broken workaround.
Admin
Regarding the author's interesting choice of variable nomenclature and strange behavior building a string one character at a time with a loop (I know know PHP, but I think that's what they're doing... right?)...
The term "secret salt" is used in cryptography as "random bits that are used as one of the inputs to a key derivation function." Check out salt in wikipedia.
As for why to build a random string one character at a time, I would guess has to do with harvesting the most entropy.
What this author is doing kinda makes sense to me as an effort to improve the quality of their random numbers, but it seems like it's just a bit of overkill to make certain of the randomness to this degree. But then again, I don't really understand the problem, so maybe there's a good reason they're doing it.
Addendum (2008-05-22 16:44): And sorry if this covers some ground that was already covered, the last thing I read was something about Pepper before I actually posted mine, and there are a lot more comments now.
Admin
Salt is a cryptographic term for randomness used in one of several cryptographic contexts: http://en.wikipedia.org/wiki/Salt_(cryptography)
Admin
Yes, while the original code may not be perfect, it is much more suitable for an environment where a web browser or other caching application may be polling this page (although, admittedly, there are simpler/better ways to do this).
Your solution is not one of the better ways. If the content of the page can change on a per-request basis, and you are serving multiple simultaneous clients, it is possible for your code to allow for cached results. For instance, many ISPs, and even small business intranets sometimes, have their own web proxies. Two clients (or one client querying the page twice) could potentially result in a cache match if you are just using time(), and result in a cached return rather than a live one. I'm not sure what context this code is called in, but I think the previous developer could have been planning for this eventuality.
I've seen this happen in production environments using exactly the kind of "fix" you put in, and it just doesn’t work well for heavily hit systems.