Safeguarding Against CSRF

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
kkonline
Forum Contributor
Posts: 251
Joined: Thu Aug 16, 2007 12:54 am

Safeguarding Against CSRF

Post by kkonline »

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


The validity of the token can also be limited to a small window of time, such as ten minutes:

I made the following some changes to post.php code is below, but it still does not go into the valid data loop, and prints "Timeup!" on pressing submit button and doesnot show any value for echo $_SESSION['token_time'];

However if i DIRECTLY go to post.php it says "Valid data!"

It's behaving just in the opposite way i expect it to.

Revised Code

Code: Select all

<?php
session_start();
$token = md5(uniqid(rand(), TRUE));
$_SESSION['token'] = $token;
$_SESSION['token_time'] = time();

?>
<form action="post.php" method="post">
<input type="hidden" name="token" value="<?php echo $token;?>" />
<p>
Symbol: <input type="text" name="symbol" /><br />
Shares: <input type="text" name="shares" /><br />
<input type="submit" value="Buy" />
</p>
</form>
post.php contains the following content

Code: Select all

<?php
if ($_POST['token']== $_SESSION['token']) {
echo "Valid data!";
exit;
}
$token_age = time() - $_SESSION['token_time'];
if ($token_age >= 600) {
// time limit can be set here as number instead
// of LOGIN_TIME_LIMIT define, such as 60*10
echo $_SESSION['token_time'];
echo "Timeup!";
exit;
}
?>
I can't figure it's behaviour as this is a very simple code. Please help simulating it's behaviour.


feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Re: Safeguarding Against CSRF

Post by superdezign »

kkonline wrote:However if i DIRECTLY go to post.php it says "Valid data!"
That is because NULL == NULL.

Hint: session_start() and isset($_POST['token']).
kkonline
Forum Contributor
Posts: 251
Joined: Thu Aug 16, 2007 12:54 am

Re: Safeguarding Against CSRF

Post by kkonline »

superdezign wrote:
kkonline wrote:However if i DIRECTLY go to post.php it says "Valid data!"
That is because NULL == NULL.

Hint: session_start() and isset($_POST['token']).
You mean i should write session_start() in the beginning of both the codes. Am i correct?
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Re: Safeguarding Against CSRF

Post by superdezign »

kkonline wrote:You mean i should write session_start() in the beginning of both the codes. Am i correct?
The $_SESSION array doesn't exist until session_start() is called. session_start() uses the user's session id to retrieve their session information.
kkonline
Forum Contributor
Posts: 251
Joined: Thu Aug 16, 2007 12:54 am

Re: Safeguarding Against CSRF

Post by kkonline »

kkonline wrote:
superdezign wrote:
kkonline wrote:However if i DIRECTLY go to post.php it says "Valid data!"
That is because NULL == NULL.

Hint: session_start() and isset($_POST['token']).
You mean i should write session_start() in the beginning of both the codes. Am i correct?
I modified the post.php as follows

Code: Select all

<?php
session_start();
if (isset($_POST['token'])==$_SESSION['token']) {
echo "Valid data!";
exit;
}
else{
echo "Wrong data!";
}
$token_age = time() - $_SESSION['token_time'];
if ($token_age >= 600) {
// time limit can be set here as number instead
// of LOGIN_TIME_LIMIT define, such as 60*10
echo "Timeup!";
exit;
}
?>
The situation

1> If i write anything on the form and submit it says Valid data OK

2> If i keep the form blank and submit it says Valid data NOT OK

3> If i directly access post.php it gives Wrong data! I think its OK

Please clarify points 2 and 3 and if the condition of time works fine if the data is valid/invalid but the time has expired.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Re: Safeguarding Against CSRF

Post by superdezign »

kkonline wrote:

Code: Select all

if (isset($_POST['token'])==$_SESSION['token']) {
Not quite what my hint meant. isset() returns a boolean value, so you are checking $_SESSION['token'] against a boolean.

I meant that you should check if $_POST['token'] isset() AS WELL as comparing the values.

Code: Select all

if (isset($_POST['token']) && isset($_SESSION['token']) && $_POST['token'] == $_SESSION['token']) {
kkonline
Forum Contributor
Posts: 251
Joined: Thu Aug 16, 2007 12:54 am

Re: Safeguarding Against CSRF

Post by kkonline »

superdezign wrote: I meant that you should check if $_POST['token'] isset() AS WELL as comparing the values.

Code: Select all

if (isset($_POST['token']) && isset($_SESSION['token']) && $_POST['token'] == $_SESSION['token']) {
I changed the code as below

Code: Select all

<?php
session_start();

$token_age = time() - $_SESSION['token_time'];
if ($token_age >= 5) {
// time limit can be set here as number instead
// of LOGIN_TIME_LIMIT define, such as 60*10
echo "Timeup!";
exit; 
}
if (isset($_POST['token']) && isset($_SESSION['token']) && $_POST['token'] == $_SESSION['token']){
echo "Valid data!";
exit;
}
else{
echo "Wrong data!";
exit;
}
?>
Thanks superdezign, It worked well.

Last question

It ALWAYS shows Timeup! wen submitted from some other site... is that ok or should it print Wrong data???
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Re: Safeguarding Against CSRF

Post by superdezign »

kkonline wrote:It ALWAYS shows Timeup! wen submitted from some other site... is that ok or should it print Wrong data???
Not sure. I do know that if a session isn't started, $token_age will equal time() - NULL. You should always check if a value isset() before using it.
kkonline
Forum Contributor
Posts: 251
Joined: Thu Aug 16, 2007 12:54 am

Re: Safeguarding Against CSRF

Post by kkonline »

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


[quote="superdezign"][quote="kkonline"]It ALWAYS shows Timeup! wen submitted from some other site... is that ok or should it print Wrong data???[/quote]

Not sure. I do know that if a session isn't started, $token_age will equal time() - NULL. You should always check if a value isset() before using it.[/quote]

Sounds good. I understand your point! Thanks a tonne. You cleared so many doubts today. Thanks again... The corrected code is

Code: Select all

<?php
session_start();

if (isset($_POST['token']) && isset($_SESSION['token']) && $_POST['token'] == $_SESSION['token'])
{

	$token_age = time() - $_SESSION['token_time'];
	if ($token_age >= 5)
 	{
	echo "Timeup!";
        exit;
 	}

echo "Valid data!";
exit;
}
else{
echo "Wrong data!";
exit;
}
?>
When submitted from some other site now it shows Wrong data as it should.

When posted from correct site within time it shows valid data if the data is correct but time is up then show Timeup!

Perfect!


feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]
Post Reply