Unsetting an unset variable
-
OK, so here's something exciting for you.
foreach (array('db_character_set', 'cachedir') as $variable) if (isset($GLOBALS[$variable])) unset($GLOBALS[$variable], $GLOBALS[$variable]);
Aside from manually having to fix line breaks when copy/pasting from Github, there are several things wrong with this.
So, first up. $cachedir and $db_character_set are being checked because at this point in execution, they might exist. They shouldn't exist - the settings file hasn't been loaded yet, but apparently they might exist and settings might not overwrite them or something.
But that's not the thing I'm getting at. Notice the unset, apparently unsetting the same thing twice.
This is actually having to work around a PHP bug.
For those not familiar, PHP's idea of arrays is to give you both a hashmap and a conventional array under the name of 'arrays'. You can freely mix indexed and hashmap even in the same array.
The whole process of making the hashmap, though, is totally hidden from the user. So when you check said hashmap for $GLOBALS['db_character_set'] it's silently converted to a hash and stored away. So far so good.
But PHP had a bug whereby colliding hashes weren't always cleaned properly with an unset meaning you could do all kinds of cute exploits with supposedly cleaned data. Oops.
-
php_real_unset(...);
-
Actually they fixed
unset()
itself. I was as shocked as you are.
-
Have you tried tracing the code to see why the settings file is loaded at that point in time? Is it something that's consistently reproducible?
-
Oh I know exactly how it came to be.
The settings file is absolutely not loaded until after that code. But there are some cases where those variables won't be set.
$cachedir
in particular didn't exist in older versions of SMF and wouldn't necessarily be set in the settings file during an upgrade but to ensure it couldn't be tainted from outside, it's unset.Personally I'd just make sure the settings file defined it and be done with it.
Also note: this is global variables (yay) and in a system that supports multiple character sets because UTF-8 was a thing but not a well supported thing when this stuff was originally written.
Large swathes of SMF 2.1 (current in-dev branch) date back to 2004.
-
Regression support in PHP, sounds like fun.
-
Actually this one wasn't a b/c breaking bug as it happened so it didn't matter that unset() suddenly started doing double duty.
-
...but to ensure it couldn't be tainted from outside, it's unset.
So basically what you're telling us is this WTFy code is meant to fix possible WTFy behavior, thus the purpose of the WTFy code is to purportedly prevent a WTF. Elegant, in the same way someone would invest in flood insurance to protect a farm in the Sahara...yes it COULD happen...
-
-
So basically what you're telling us is this WTFy code is meant to fix possible WTFy behavior, thus the purpose of the WTFy code is to purportedly prevent a WTF. Elegant, in the same way someone would invest in flood insurance to protect a farm in the Sahara...yes it COULD happen...
Well... sort of.
People do shitty things in PHP, especially when register_fucking_globals was still a thing because you could pass crap in via the URL and automatically have it generate fucking global variables. As a result, these could be tainted.
It's a WTF that the first variable never got sanitised before use, granted. The second, I'm a little bit less annoyed with because 1.x of the software never used $cachedir, that was a 2.0 only thing, but 2.0's upgrader didn't bother to attempt to force $cachedir to be set in Settings.php, meaning that potentially there's another variable that could be tainted. Either or both variables may be set in the settings file, but it's quite possible neither would be.
So the only thing they can do is nuke both from orbit and then if they're declared after that, they're declared safely. The only reason this is a WTF is because PHP itself required you unset shit twice because it couldn't do it properly the first time.
-
The only reason this is a WTF is because PHP itself required you unset shit twice because it couldn't do it properly the first time.
Yeah, they should have required you to do it three times. For reasons of poetry.
-
Yeah, they should have required you to do it three times. For reasons of poetry.
Well, PHP fixed it now, some modicum of sanity prevails.
-
while (isset($GLOBALS[$variable])) { unset($GLOBALS[$variable], $GLOBALS[$variable]); DoEvents; }
-
What, I don't even...
dammit we don't need more bad PHP code
-
Hopefully, you haven't touched pre-.NET VB.
-
Hopefully, you haven't touched pre-.NET VB.
I once wrote a replacement taskbar for Win98 in VB6.
-
Sounds like an early version of UltraMon.
-
Sounds like an early version of UltraMon.
In my naivety, I figured if I had something written in VB6 already loaded, I could add future apps without having to load extra copies of the VB runtime DLLs in memory.
Fortunately in the 15 years since that nonsense, I've learned from it.
-
VB .NET!
Set It And Forget It™!
-
Hopefully, you haven't touched pre-.NET VB.
Too late. I wrote a key code generator program for a lock company that could calculate master and grandmaster keys in VB 3 (yes, THREE, as in SIXTEEN BIT VB) back in '97. Still bear the scars. Although it actually did work (if the memory didn't overflow onto the hard drive - a wtf for another discussion), I still have nightmares about it eventually showing up on this forum someday. ;-)
-
I once wrote a replacement taskbar for Win98 in VB6.
I take it that it was no laughing matter?
-
I take it that it was no laughing matter?
Great exercise in how to call the WinAPI a lot from Visual Basic. But mostly pain and misery from a teenager that thought he knew better.
There is a reason I have never gone back to VB for desktop stuff since. VBA, sure, VBScript/ASP Classic, sure, but I have never been back to Visual Basic itself since.
-
Any sort of VB besides .NET needs to die a painful death.
-
-
So, all of these, too?
-
Now just take that and put it in a function called
real_unset
and you'll have a most elegant solution.
-
-
VBulletin can't possibly be more hated by us than Discocurse, can it?
-
VBulletin can't possibly be more hated by us than Discocurse, can it?
When vB 5 shipped the day before the vB/XenForo lawsuit was over, it was missing minor details like a list of members, a warning system, a FAQ system (which had been a feature since the vB 3 days) and a ton of other stuff, not to mention many many bugs, including running 60+ queries PER PAGE.
-
And also enough XSS vulnerabilities to make Discourse look like an iron curtain.
Then again, Community Server still takes the biscuit.
-
That's because vB 5 tossed out the mostly stable and mostly hardened 4.x series which also tossed out the mostly stable and mostly hardened 3.x series.
-
Looks like the code might have hardened over time, but the developers haven't. They're still making the same schoolboy errors.
http://what.thedailywtf.com/t/how-much-of-a-wtf-is-vbulletin-5/2132
-
Of course they are because it's all fucking outsourced.