Hiding $_GET variables using encryption

I use the following functions to encrypt my $_GET variables (whenever I can't easily get away with using $_POST or some other way of passing information between pages).

function decryptStringArray ($stringArray, $key = "Your secret salt thingie")
{
    $s = unserialize(rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, md5($key), base64_decode(strtr($stringArray, '-_,', '+/=')), MCRYPT_MODE_CBC, md5(md5($key))), "\0"));
    return $s;
}

function encryptStringArray ($stringArray, $key = "Your secret salt thingie") 
{
    $s = strtr(base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, md5($key), serialize($stringArray), MCRYPT_MODE_CBC, md5(md5($key)))), '+/=', '-_,');
    return $s;
}

function prepareUrl($url, $key = "Your secret salt thingie")
{
    $url = explode("?",$url,2);
    if(sizeof($url) <= 1)
        return $url;
    else
        return $url[0]."?params=".encryptStringArray($url[1],$key);
}

function setGET($params,$key = "Your secret salt thingie") 
{
    $params = decryptStringArray($params,$key);
    $param_pairs = explode('&',$params);
    foreach($param_pairs as $pair)
    {
        $split_pair = explode('=',$pair);
        $_GET[$split_pair[0]] = $split_pair[1];
    }
}

Obviously I replace the "Your secret salt thingie" with other strings. Here is how I use it:

On the page where I need a URL:

$url = prepareUrl("http://someurl.com?variable1=1314&variable2=1851&variable3=stringstuff", "algjalgjalgjal");

Then I put the new $url in a href or a tag or something (I use $smarty templates but that isn't relevant).

On the page someurl.com where I need to decrypt the params I just use:

setGET($_GET['params'],"algjalgjalgjal");

This all works fine for me. Is there anything inherently terrible about this way of doing things? I'm asking this because I posted this as an answer on Stack Overflow to a question someone asked about hiding their $_GET parameters and it was immediately down-voted. That made me curious about whether

Agree 100% with Corbin. GET isn't something that is inherently secure, and trying to make it so is nigh impossible. That's what POST is for. Only use GET for non-sensitive information. That being said, there are some generally concerning bits to your code that could cause some people to downvote it.

Your line length is rather long and convoluted, which causes major issues with legibility. I would suggest breaking up that single line into multiple lines to more easily maintain and read. Adding whitespace couldn't hurt either. Additionally, using single letter variables is not very descriptive and also leads to issues with legibility. Its a bit longer, but much easier to read.

$translation = strtr( $stringArray, '-_,', '+/=' );

$key     = md5( $key );
$data    = base64_decode( $translation );
$iv      = md5( $key );
$decrypt = mcrypt_decrypt(
    MCRYPT_RIJNDAEL_256,
    $key,
    $data,
    MCRYPT_MODE_CBC,
    $iv
);

$trimmed = rtrim( $decrypt, "\0" );

return unserialize( $trimmed );

A couple more potential issues with the above code are the methods you are implementing. I am by no means a security guru and have not put a lot of research into the matter, but I seem to remember from somewhere that mcrypt is frowned upon and that hashing a string with md5() twice is actually less secure than doing it just once. I don't know if this is true or not, but that could be a potential issue.

Your code is also slightly repetitive, violating the "Don't Repeat Yourself" (DRY) Principle. As the name implies, your code should not repeat. Your encrypt and decrypt functions seem to share some common elements, using some shared helper functions to provide that similar data might be beneficial, even though you might just end up creating wrapper functions. I don't really see anyone downvoting you for this alone, this is rather minor in this instance and is rather hard to spot due to the above reasons.

Another potential issue I see is with your braceless syntax. Braceless {} syntax can be somewhat confusing to those who have never seen it before, and therefore could cause issues with maintainability. This is entirely a point of preference, but one I would argue most vehemently against. It is especially bad to offer in the form of an answer when it is unknown how the questioner will use it. It is quite possible they attempted to modify the code and could not get it to work.

The last issue I see is the way you are accessing array elements with magic numbers. This is rather sloppy and could cause issues with legibility, though I don't think you would get downvoted for it. There are a number of different ways this could be solved. The first is by using the PHP construct list(), which is probably preferable in this instance. The second is by using array functions to slice off the required portions of the array. This is more beneficial when you need the first and last elements of an array of undetermined length. Finally, there is also the possibility of using extract(), but that requires an associative array and is sometimes frowned upon. I won't show that last method because it doesn't apply here, but here are the other two:

//using list
list( $baseurl, $params ) = $url;

//using array functions
$baseurl = array_shift( $url );
$params  = array_shift( $url );//could potentially use array_pop()

return $baseurl . '?params=' . encryptStringArray( $params, $key );

Hope this helps!

StackExchange Code Review Q#20257, answer score: 6