I'm trying to develop a form helper to prevent form spoofing. So i came up with this:
<form...>
<?=form::secure()?>
...
</form>
which stamps an hidden form with key '_token' with a token which is the md5 of the id of the user session (which is random and renewed after 1 week).
And then at the "action" url (from the form):
$token = (isset($_GET['_token'])) ? $_GET['_token'] : null;
$token = (is_null($token) and isset($_POST['_token'])) ? $_POST['_token'] : $token;
if (form::is_secure($token)) { // checks if the given token is equal to md5(user id)
...ok...
} else {
...error...
}
Does that prevent form spoofing or I am missing something? In case the user id expires in the exact moment when the user has just loaded the page and then submit its form, it will just print an error and he should submit the form again, but thats acceptable (it's rare).
I thought that the only thing that could go wrong here is if the potential attacker is able to get the user session id and then he can attach ?_token=id to it's request and give it to the user to browse, but at that point, if the attacker has the user session id it can do whatever he wants anyway.
Am I right? If not, how can I edit my code to accomplish my goal?
You are effectively reinventing the wheel, since the token requires a valid session to validate and thus only repeats the security layer of sessions while not adding anything extra. It is the equivalent of asking for someone to enter their password in and then asking for it again to be sure. It may make everyone feel better, but if the password has been compromised, it only slows down the attack but doesn't prevent it.
This isn't to suggest that you shouldn't take these things seriously or to dismiss your attempts. It is my philosophy that the content of the form (What is passed in via GET
or POST
) should be separated from other logic, such as security and calculations, and that the server should not consider user-supplied data as anything other than user-supplied data. Ideally anyone could post anything to a forms action, and the server/controller will manage it correctly and consistently. The session is verified (security), the data is sanitized and then validated, and any values that are generated via JS when the user is actually using the web form is recalculated/verified. The response, ideally, is generic enough, either a redirect or web-standard response, so that the requester, be it a web browser, command line user, or web service can interpret it.
If the above were the case, there would be less emphasis and concern of spoofing, more emphasis on enhancing security layers and validation layers separately, and, best of all, the back-end controller that did this well could easily be ported/reused as a web service backend.
Long story short: Your solution isn't bad, it just isn't adding any real security as far as I can tell, and may even expose your back-end security logic. If you have a specific spoofing concern/threat, it would be better to address that use case and work your way out rather than try to come up with a one-size-fits all solution right away. It may turn out that your use cases have specific solutions/considerations that need to addressed at different points of the exchange.
Don't reinvent the wheel. If you're not using a web framework, integrate one of the myriad libraries that add CSRF protection to your project such as CSRF4PHP.
Your solution is close, but wouldn't work because the session ID would be scrapable by an attacker.