BEWARE THE "BLACK_VARS"!!!



  • For the first time since starting my job six months ago I found myself literally banging my head against the desk.

    After wading through uncommented spaghetti code for over an hour I finally came on this line, which was the proverbial straw:

     

        for ($i = 0; $i < count($BLACK_VARS); $i++) {

            $control->$BLACK_VARS[$i] = $hidden->$BLACK_VARS[$i];
        }

     

    The "control" object has no such property (nor does the hidden object), and in the file I'm working with these are the only references to this mysterious array. 

    The name ($BLACK_VARS) kind of sums up how I feel the previous developer must have felt about most of their programming. It all was just sort of "black magic", copy this here, do that thing there, hold your tongue just right and cross your fingers, say the magic words, and maybe, just maybe, it'll all somehow work...



  • Hmm.. if that's PHP it's complete junk.  $BLACK_VARS is presumably an array (because of the count) but it's being used as a property-name in control, which wouldn't even work.  In other words, the property of $control wouldn't be named BLACK_VARS, but instead array.  Unless it's a typo on your part.



  • Was that supposed to be more like:

    [code] for ($i = 0; $i < count($control->BLACK_VARS); $i++) {

        $control->BLACK_VARS[$i] = $hidden->BLACK_VARS[$i];
    } 
    

    [/code]

    ???



  • Yeah, it's PHP.

     

    No typos, I copied and pasted that code.

     

    What I've found is that this is how they like to call functions, set properties, etc. For instance, they will submit a form that sends a "function call" like so:

    URL something like:

    somePage.php?runs=1&mops=34&stns=free

     

    Code at URL is something like: 

    $KOWR = array("GENERAL", "REPORTS", "GEO", "ZIPSGET");

    $body = $KOWR[$runs]($mops, $stns);

    //snip 100 lines of html echoed out

    echo "                <td><center>" . $body . "</center></td>";

     

    Then the code in some random included by an included include file is something like:

    //at about line 1500:

    function REPORTS($mops, $tetras){

    //snip about 100 lines of HTML concatinated line by freaking line with nested upon nested un-indented if statements

    return $body; 

    }

     

    And none of it is really comented at all. Well, it is, but it doesn't really mean anything to me. Comments pretty much go like:

    //for pre-defined, we need to take what is in the hidden element rather than the control -- control is used to update with pKeys (function updateFields) 

    Which is guess is marginally better than nothing... (note that the above comment immediately preceeded the "BLACK_VARS" if check)



  • @cmcculloh said:

    What I've found is that this is how they like to call functions, set properties, etc. For instance, they will submit a form that sends a "function call" like so:

    Skull-crackin' time!



  • @cmcculloh said:

    What I've found is that this is how they like to call functions, set properties, etc. For instance, they will submit a form that sends a "function call" like so:

    URL something like:

    somePage.php?runs=1&mops=34&stns=free

    Sadly, I am familiar with this design pattern.  In addition to the "pass the name of the table you will be querying as a GET var" and "have one 5000-line function that takes an 'action' parameter rather than five 1000-line functions" patterns... :`-(



  • @shadowman said:

    Was that supposed to be more like:

    <font face="Lucida Console" size="2"> for ($i = 0; $i < count($control->BLACK_VARS); $i++) {

        $control-&gt;BLACK_VARS[$i] = $hidden-&gt;BLACK_VARS[$i];
    } 
    
    </font>

    ???

     

     

    No, see, that would actually make sense. This code actually presumably works as intended, and would be considered "correct" by it's author (and, yeah, the page actually works correctly). I have no freaking clue what this code does yet, but I'm sure I'll figure it out. $BLACK_VARS is an array that I'm sure I will find buried within some included file somewhere. This sort of thing is exactly what led me to find this code block in the first place. I had another array that was being used in this sort of way in another file to call a function that used a variable set by a function called by the $control object, that was being used by another file included in this base file, which I'm sure will in turn lead me to whatever file this BLACK_VARS array crawled out of (hope that made sense). It's a mess. I'll post when I know what is actually going on...



  • Looks to me like they're using [url=http://us2.php.net/manual/en/language.variables.variable.php]variable variables[/url], one of the rather peculiar features of PHP (though useful in some rare circumstances). This is how it works:

    class foo
    {
       var $bar;
       var $baz;
       function foo() { $this->bar = 'bar'; $this->baz = 'baz'; }
    }
    
    $foo = new foo();
    
    $var = 'bar';
    $foo->$var = 'omg';
    $var = 'baz';
    $foo->$var = 'wtf';
    
    // now $foo->bar is 'omg' and $foo->baz is 'wtf'
    

    Basically, they're copying the properties listed in $BLACK_VARS from $hidden into $control (e.g. if $BLACK_VARS = array('location', 'time', and 'wtf'), it'll do "$control->location = $hidden->location; $control->time = $hidden->time; $control->wtf = $hidden->wtf;").



  • @Quietust said:

    Looks to me like they're using variable variables, one of the rather peculiar features of PHP (though useful in some rare circumstances).

    So it's a way to reference class properties dynamically by name?



  • @AbbydonKrafts said:

    So it's a way to reference class properties dynamically by name?

    In this case, that's what they're using it for. In practice, it can be used to reference [i]any[/i] variable dynamically by name (e.g. "$a = 'var1'; echo $$a;' will do 'echo $var1;').



  • I can see the rare need to dynamically reference class properties as I have done that before in other languages, but to dynamically reference a sole variable? How odd..



    Disclaimer: I have not used PHP, but I'm trying to pick up on PHP-related topics as I'd like to use it in Linux (which I'm just starting on learning).



  • @AbbydonKrafts said:

    but to dynamically reference a sole variable? How odd..
     

    Think of it as a very bad attempt at recreating C-style pointer dereferencing in a scripting language where memory addresses are generally completely hidden. Like with everything in PHP, someone found it useful at some point and glued it in somewhere with some ducttape.



  • The only reason I can imagine for something like this is that they found they didn't have enough security vulnerabillities yet...



  • @Quietust said:

    Looks to me like they're using variable variables, one of the rather peculiar features of PHP (though useful in some rare circumstances). This is how it works:

    class foo
    {
       var $bar;
       var $baz;
       function foo() { $this->bar = 'bar'; $this->baz = 'baz'; }
    }
    

    $foo = new foo();

    $var = 'bar';
    $foo->$var = 'omg';
    $var = 'baz';
    $foo->$var = 'wtf';

    // now $foo->bar is 'omg' and $foo->baz is 'wtf'

    Basically, they're copying the properties listed in $BLACK_VARS from $hidden into $control (e.g. if $BLACK_VARS = array('location', 'time', and 'wtf'), it'll do "$control->location = $hidden->location; $control->time = $hidden->time; $control->wtf = $hidden->wtf;").

    See, I got that it would be using variable variables (as written) but that seemed so wrong for so many reasons, so I assumed the $ before BLACK_VARS had to be a typo.  Why would you copy properties like this?  And why would you store them in an array named BLACK_VARS?  And why would you use a for loop instead of foreach?  I honestly believe PHP is a great language, but it gives you too much ropte to hang yourself with. 


Log in to reply