Forum Moderators: coopster

Message Too Old, No Replies

Help needed with php script for style demo

auth's & securing cookie problems

         

ChaosBurnt

11:55 am on Dec 1, 2007 (gmt 0)

10+ Year Member



Hello, this is my first post here, I am asking for some help please.
I have a phpbb style site where I create new and original styles for people to download free of charge. I am trying to get a site-wide style demo to work on my styles forum. So far the style demo works perfectly if users are logged in but the style demo doesn't work if the user is not logged in. I am 'very' new to php and I am having great difficulty in understanding what the auth's problem is.

This is the script used:

In the index.php:

//------ change theme on index

$fpage_style = $userdata['user_style'];
if(isset($HTTP_POST_VARS['fpage_theme']))
{
$fpage_theme = intval($HTTP_POST_VARS['fpage_theme']);
$fpuser_id = $userdata['user_id'];
$fp_sql = "UPDATE " . USERS_TABLE . " SET user_style = '$fpage_theme' WHERE user_id = $fpuser_id";
if (!($fp_result = $db->sql_query($fp_sql)) )
{
message_die(GENERAL_ERROR, 'Could not update users table ' . "$user_id $fpage_theme", '', __LINE__, __FILE__, $sql);
}
else
{
$redirect = ( isset($start) )? "&start=$start" : ''; redirect(append_sid("style_demo.$phpEx" . $redirect, true));
$fp_message = $lang['Profile_updated'];
message_die(GENERAL_MESSAGE, $fp_message);
}
}
//------ change theme on index

In the common.php:

//------ change theme on index

if ($template)
{
$board_config['default_style'] = $template;
setcookie('default_style', $template , (time()+1600), $board_config['cookie_path'], $board_config['cookie_domain'], $board_config['cookie_secure']);
}
else if (isset($HTTP_COOKIE_VARS['default_style']) )
$board_config['default_style']=$HTTP_COOKIE_VARS['default_style'];
//------ change theme on index

In the style overall_header.tpl:

<table width="100%" cellspacing="0" cellpadding="0" border="0" align="center" margin="0">
<tr>
<td align=center valign=middle><span class="gensmall">
<!-- BEGIN switch_user_logged_out -->
<form method="post" action="{U_INDEX}"><b>{L_SELECT_STYLE}</b>
<br>{TEMPLATE_SELECT} <input type="submit" class="mainoption" name="changenow" value="{L_CHANGE_NOW}" />
</form>
<!-- END switch_user_logged_out -->
<!-- BEGIN switch_user_logged_in -->
<form method="post" action="{U_INDEX}"><b>{L_SELECT_STYLE}</b>
<br>{FPAGE_STYLE} <input type="submit" class="mainoption" name="fpchangenow" value="{L_CHANGE_NOW}" />
</form>
<!-- END switch_user_logged_in -->
</span></td
</tr>
</table>

I am completely clueless as to why the style demo doesn't work unless users are logged in. I wonder if anyone here can see why the not logged in condition is not allowing the index to change style.
It was also pointed out to me at a style support forum that the $HTTP_COOKIE_VARS['default_style'] isn't secured. I have searched for hours on end on numerous php security related sites and I still do not understand why this isn't secure and I don't know how I am meant to secure it. So I would also very much appreciate it if anyone here understands what is meant by that isn't secure and they know how to secure if they were to tell me why it isn't secure and how I secure it.

Please allow me to stress again that I am 'very' new to php coding and simple help and advise would be highly appreciated.
Thank you very much for taking the time to read my topic.
Regards,
ChaosBurnt.

PHP_Chimp

7:49 pm on Dec 1, 2007 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Although this first bit may well not sort your problems it will help you tidy up the code little.
As opposed to $HTTP_POST_VARS use $_POST. They do (almost) the same thing, but $_POST is quicker to write.

Be careful when converting from one type to another. As with -

$fpage_theme = intval($HTTP_POST_VARS['fpage_theme']);

You are taking whatever is in the post fpage_theme and forcing it into an interger. If the string is already a number then dont worry about changing it over, as php is smart enough to do that for you (generally). As if you force letters into numbers then you may well not get the result you were expecting.

$_COOKIES (or $_HTTP_COOKIE_VARS in your case) are not secure as any person can write there own cookies to there hard drive, so I could choose something nasty to put into the cookie and when you site reads that cookie I do bad stuff to your server. So whenever you are accepting data that a user has entered you should always assume that they are going to kill your server. Make sure that the data is what you expect, so in your case you may well be able to say something like -


switch($_COOKIE['default_style']) {
case 'style 1':
case 'style 2':
case 'list all of my styles':
//ok
break;
default:
die ('go on punk make my day');
break;
}

Security is always relative, but seeing as you are likely to have records of people email addresses you dont want people to be looking to hack into your server. There are lots of good discussions about securing your scripts on this forum so you could search for them...or just start a new one.

So back to your problem -

$fpuser_id = $userdata['user_id'];

Is requiring a user_id, so im guessing that you dont have a default user_id for people that are not logged in?
You could always give a 'guest' id to people that are not logged in, then a 'user' id to those that are logged in. Depending on how you have assigned your user id's you may be able to use 2 as the id for guests (try to avoid 0 and 1, as they can also be true/false, so you need to use === if you are testing for 0 or 1, its another small security vulnerability if you only use == not === to test for 0 or 1)

There may be other issues with the script, but this will give you a start.

O ye, welcome to webmaster world :)

ChaosBurnt

3:34 pm on Dec 3, 2007 (gmt 0)

10+ Year Member



Thanks for your great response and the kind welcome PHP_Chimp.

I'll have to chew your answer over and get my mate Steve to have a look at it too. Steve's the one who's better at php than myself, I really am a newbie to it all and I'm struggling to figure it all out. My forte is grahic design to be honest and I'm not the best at that. Hopefully between us Steve and myself will get it all sorted in the end. Steve's got the demo to operate and he's used the following to secure the cookie:

//-- ADD mod : Styles DEMO -----------------------------------------------------
if ( $template )
{
$board_config['default_style'] = $template;
setcookie('default_style', $template , (time()+1600), $board_config['cookie_path'], $board_config['cookie_domain'], $board_config['cookie_secure']);
}
else if ( isset($HTTP_COOKIE_VARS['default_style']) )
{
$board_config['default_style'] = trim(htmlspecialchars($HTTP_COOKIE_VARS['default_style']));
}
//-- ADD mod : Styles DEMO ----------------------------------------------------

Does that all look right to you?

PHP_Chimp

6:33 pm on Dec 3, 2007 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Sending a cookie [uk3.php.net]as secure will mean that it is only sent over https, not over http. If you have PHP 5.2 then you can use http only for the cookie.
Quote from the manual about http only for cookies
When TRUE the cookie will be made accessible only through the HTTP protocol. This means that the cookie won't be accessible by scripting languages, such as JavaScript. This setting can effectly help to reduce identity theft through XSS attacks (although it is not supported by all browsers). Added in PHP 5.2.0. TRUE or FALSE

Using htmlspecialchars is a good idea, as this will help to stop people putting 'interesting' code into your cookies. However unless you have a huge number of values that you are expecting for the cookie you may well be better off searching to make sure that the cookie actually contains those values, and if it doesnt then not using it. As you know the what values should be in that cookie.
Its not like you are using a cookie to store there user name, as that would be almost impossible to access if the value was a correct value. Without doing a look up in your user name table, and if the cookie is going to be used for sql injection then using the value as part of a query is not what you want.

The only other thing is that the expiry time is in seconds, so you have 1600 - that is just over 26 mins. Is that actually what you wanted?
To make it easier you can use the strtotime [uk3.php.net]function, as then you can say strtotime(+1 month).

It would also help you to use $_GET¦POST¦COOKIE¦FILE¦SERVER as opposed to the longer $HTTP_*_VARS. Have a look at the section on reserved variables [uk3.php.net] in the manual, as it explains the difference.

ChaosBurnt

11:57 am on Dec 4, 2007 (gmt 0)

10+ Year Member



Hi again PHP_Chimp, thanks for your answer, I really appreciate you granting some of your time, thank you.

I've linked Steve here as he said the information you gave me was very good advice and he wants to know more about the site here so he's going to come over at some point.

Yeah, the +1600 is intentional, we only want to give users the time to view a given style long enough for them to decide if they want to access it in the downloads area.
Rather than having the age-old screenshot demo's we are trying to give users a fuller and more informative experience via the style demo mod'. I'll let Steve know your latest response, some of the standard php stuff pertaining to older phpbb is not relevant to CH php due to how Pierre is coding CH (CH is a modified version of phpbb2 called categories hierarchy written by a guy called Pierre, we're on his site trying to get this style demo running fully!).
Hopefully Steve will be able to make it over soon, I'll link him to this topic so he can contact you directly. This is in all honesty way over my head, Steve will be able to talk better with you than I can.
Back later mate ;-)

Stevec4

3:12 pm on Dec 4, 2007 (gmt 0)

10+ Year Member



Hey Guys

Php_chimp thanks for all the help with this mod. I have made the changes you suggested using the super globals. I also hard coded the user_id of the my test user. Everything is working well with the mod. If I dump the user it is showing user 9. But still if the user is not logged in the style never changes. If they are logged in it changes fine. As CB said it is Category Hierachy Mod, which uses a AUTHS scheme to secure the board. I am beginng to think that is what is holding us back.

Here is the latest code snippets. Showing the changes. Once again I sure appreciate all the feed back you provided to help get this working.

styles_demo.php

$fpage_style = $userdata['user_style']; 
if ( isset($_POST['fpage_theme']) )
{
$fpage_theme = _read('fpage_theme', TYPE_INT);
$fpuser_id = $userdata['user_id'];
$fp_sql = 'UPDATE ' . USERS_TABLE . '
SET user_style = ' . $fpage_theme . '
WHERE user_id = ' . $user_id = 9;
if (!($fp_result = $db->sql_query($fp_sql)) )
{
message_die(GENERAL_ERROR, 'Could not update users table ' . "$user_id $fpage_theme", '', __LINE__, __FILE__, $sql);
}
else
{
$redirect = ( isset($start) )? array('start' => $start) : ''; redirect($config->url('style_demo', $redirect, true));

$fp_message = $lang['Profile_updated'];
message_die(GENERAL_MESSAGE, $fp_message);
}
}

common.php

//-- ADD mod : Styles DEMO -----------------------------------------------------
if ( $template )
{
$board_config['default_style'] = $template;
setcookie('default_style', $template , (time()+600), $board_config['cookie_path'], $board_config['cookie_domain'], $board_config['cookie_secure']);
}
else if ( isset($_COOKIE['default_style']) )
{
$board_config['default_style'] = trim(htmlspecialchars($_COOKIE['default_style']));
}
//-- ADD mod : Styles DEMO ----------------------------------------------------

The time is intentional btw since it is a styles demo I just wanted a short period that the demo would be changed then back to default.

Steve