I take security pretty seriously because I’ve had about a handful of sites hacked on my watch (luckily none of it was code that I’d written). Over the years I’ve gotten used to programming with security in mind and there are a few PHP ‘nevers’ that I don’t do any more.

A few days ago I read a post on Browie.com about rotating affiliate marketing offers. The logic behind this is great and everyone in affiliate marketing should be doing this, so kudos to Browie for reminding us and showing the noobs how it’s done. However, I noticed a glaring PHP security flaw that I wanted to point out.

Here is the code that Browie gives as an example:

<?php
extract($_REQUEST);
// offer.php
$offer[1] = “affiliatelink.com”;
$offer[2] = “affiliatelink.com”;

$goto = $offer[rand(1, sizeof($offer))];

header(”Location: $goto”);
?>

The problem lies in the first line of code. One thing I have learned is NEVER do an extract() on $_REQUEST, $_GET, $_POST or any other global variable. In this particular case I’m not really sure why extract() is being used or why the $_REQUEST var is needed, but I do know that it opens up the script to be used as a jump page for anyone (albeit not really reliable). For instance, if you knew the location of the jump page, which you could find out from the ad link, then all you would have to do is construct a url like example.com/jumppage.php?offer[3]=http://mylink.com&offer[4]=http://mylink2.com and your urls would be added to the $offer array and be randomly jumped to. So unless you are super evil and can think of a way to really exploit this (messing with their stats?), then this isn’t a huge security risk. However, the point is that it allows anyone to inject variables into the script and make the script do things it was not intended to do. So even though the risk in this script is minimal, it’s best to just avoid it all together so that it doesn’t start to become a habit. Case in point: Just last week I came across some custom third party code on a client site that allowed me to login to their system without a username or password because of a combination of doing an extract() on $_REQUEST and not escaping variables in an sql query. Not a very fun thing to find.

In case you are wondering, here is how I would rewrite the script:

<?php
$offers = array('http://example.com/offer1.html',
                'http://example.com/offer2.html',
                );
header('Location:'.$offers[array_rand($offers)]);
?>

Notice that the $_REQUEST variable is missing since I don’t see why it is needed, but if you have to use $_REQUEST variables then make sure that you call them like so:

$get_variable = $_GET['get_variablename'];
$post_variable = $_POST['post_variablename'];
$lazy_variable = $_REQUEST['get_or_post_variablename'];

I suggest you use either $_GET or $_POST since you should know where your variables are coming from (ie don’t be lazy).