Hiding $_GET variables using encryption

Question

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 it was somehow bad code or insecure in some way.

Answer 1

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!

Answer 2

So far I think you are doing a decent job of obfuscation, if that's what you want.

If only your server is supposed to see the data, then it should stay on your server. Your server will have some index of data (session? database?) mechanism for identifying which data the client is working with, and only send the client the index to the data, not the data itself.

BUT based on your comment about wanting to obfuscate the indices, maybe some more validation logic on your server would be the best solution. If you don't want the user to mess with the query string to do something, then come up with rules that the server can use to determine whether access is valid at that time and allow the user to create his own query strings if he wants to.

You can't build a truly secure website until you can allow advanced users to access your server by any means they choose, including constructing raw HTTP requests.