Page 2 of 2
Posted: Mon Nov 07, 2005 4:32 pm
by Ambush Commander
Well, according to your current code, there is no way for cookies to be set. Therefore, you'd have to "login" for every page you wanted to view. Somethings missing from the picture.
And yeah, sessions are a good idea.
Posted: Mon Nov 07, 2005 5:18 pm
by foobar
Dark[NSF] wrote:foobar wrote:Dark[NSF] wrote:
for SQL it doesn't, but it will prevent some javascript on random html tags, or some XSS.
Which has nothing to do with your question. Ambush commander already gave you the answer.
I was refering to security as a whole, strip_tags() is probably a good place to start. i'm not saying he's wrong.
Oh allright. It just seemed like you were pulling stuff from the blue. Confused me a bit.
Posted: Mon Nov 07, 2005 5:48 pm
by Dark[NSF]
Ambush Commander wrote:Well, according to your current code, there is no way for cookies to be set. Therefore, you'd have to "login" for every page you wanted to view. Somethings missing from the picture.
And yeah, sessions are a good idea.
alright, here you go. why should i use sessions? just for security purposes?
Code: Select all
// Login to Account
function acc_login()
{
if ( ( isset( $_POST[ 'login' ] ) ) && ( isset( $_POST[ 'password' ] ) ) )
{
if ( bverify_login(0) )
{
setcookie( "tj_email", $_POST[ 'login' ], time( ) + 36000 );
setcookie( "tj_password", $_POST[ 'password' ], time( ) + 36000 );
header( "Location: account.php?message=login_correct" );
}
else
header( "Location: account.php?message=login_incorrect" );
}
else
header( "Location: account.php?message=login_blankfield" );
}
// log me out
function acc_logout( )
{
setcookie( "tj_email", null, time( ) - 36000 );
setcookie( "tj_password", null, time( ) - 36000 );
header( "Location: index.php?message=logout" );
}
Posted: Mon Nov 07, 2005 6:52 pm
by Sequalit
1. Security.
2. Users may have cookies disabled.
Posted: Mon Nov 07, 2005 7:55 pm
by Ambush Commander
Well, point two is mostly null considering how sessions usually work.
I suggest you first read up on what sessions actually are:
Session Handling Functions and $_SESSION
Here's how I go about authentication. It's not usable in its current state, but it should help you get some ideas.
I view authentication as a four question process:
1. Is the user logging out?
2. Is the user logging in?
3. Does the user have a valid session?
4. Does the user have a remember me token?
Each of these questions is associated with a particular action that determines whether or not it's valid, and then what to do in response.
Code: Select all
<?php
//directly handles input... the input should be passed to it via the input
//controller, and this be part of the ApplicationController that figures out
//whether or not a special command has to be issued.
class Filter_Authentication extends Filter
{
var $_plugin;
var $_mapper_user;
function Filter_Authentication() {}
function execute(&$registry) {
parent::execute(&$registry);
$this->_plugin =& $this->_registry->getPlugin();
$this->_mapper_user =& $this->_registry->getMapperUser();
if($this->assertLogout() && $this->executeLogout()) return;
if($this->assertLogin() && $this->executeLogin()) return;
if($this->assertSession() && $this->executeSession()) return;
if($this->assertRemember() && $this->executeRemember()) return;
$this->_registry->setSession(new Session_Null());
}
//these look like application states
function assertLogout() {
return isset($_POST['logout']);
}
function assertLogin() {
return isset($_POST['user']) && isset($_POST['pass']);
}
function assertSession() {
return isset($_COOKIE[$this->_plugin->getCookieSession()]);
}
function assertRemember() {
return isset($_COOKIE[$this->_plugin->getCookieToken()]);
}
//these look like commands
function executeLogout() {
session_start();
header('Cache-Control: private, pre-check=0, post-check=0, max-age=0');
if ($this->assertRemember()) {
$cookiebits = $this->parseTokenCookie();
if ($cookiebits) {
$MapperToken = $this->_registry->getMapperToken();
$token =& $MapperToken->find($cookiebits[1]);
if ($token && ($token->getValue() === $cookiebits[2])) {
$token->delete();
}
}
setcookie($this->_plugin->getCookieToken(),'',0);
}
$session =& Session::newFromSession();
if ($session) {
$session->delete();
$this->_registry->setSession(new Session_Null());
}
Template::refresh('logout');
return true;
}
function executeLogin() {
if (empty($_POST['user']) || empty($_POST['pass'])) return false;
if (!arep($_POST['user'])) return false;
$user =& $this->_mapper_user->findByName($_POST['user']);
if (!$user) return false;
if (!$user->authenticate($_POST['pass'])) return false;
session_start();
header('Cache-Control: private, pre-check=0, post-check=0, max-age=0');
$session =& Session::newFromUser(&$user);
$this->_registry->setSession(&$session);
$session->update();
Template::refresh('login', &$user);
return true;
}
function executeSession() {
session_start();
header('Cache-Control: private, pre-check=0, post-check=0, max-age=0');
$session =& Session::newFromSession();
if (!$session) return false;
if ($session->isExpired()) {
$session->delete();
return false;
}
$session->setLastaccess();
$session->update();
$this->_registry->setSession(&$session);
return true;
}
function executeRemember() {
$cookiebits = $this->parseTokenCookie();
if (!$cookiebits) return false;
$MapperToken = $this->_registry->getMapperToken();
$token =& $MapperToken->find($cookiebits[1]);
if (!$token) return false;
if ($token->getValue() !== $cookiebits[2]) return false;
session_start();
header('Cache-Control: private, pre-check=0, post-check=0, max-age=0');
$session =& Session::newFromUser($token->getUser());
$this->_registry->setSession(&$session);
$session->update();
return true;
}
//this looks like domain logic
function parseTokenCookie() {
$cookie = $_COOKIE[$this->_plugin->getCookieToken()];
$cookiebits = explode('-', $cookie);
if (sizeof($cookiebits) != 3) return false;
if ($cookiebits[0] != '1') return false; //version number
if ($cookiebits[1] == '' || $cookiebits[2] == '') return false;
if (!is_numeric($cookiebits[1])) return false;
return $cookiebits;
}
}
?>
Posted: Fri Nov 25, 2005 1:04 am
by Jenk
When dealing with the Database, no strip_tags() has nothing to do with it. That is the job of mysql_real_escape_string()
strip_tags() is for output that is being sent to the user agent. IMO it's better practice to use htmlemtities().
Another stage you might want to consider for authentication or validation is the email address the user enters. A simple regex function to determine if the address is in valid email address format

Posted: Fri Nov 25, 2005 4:10 am
by Maugrim_The_Reaper
strip_tags is essentially useless in security except in very limited and predictable scenarios. Not in everything, but largely a 10 year old can input a string which neatly avoids stripping from that function. As a general rule, if something is not supposed to contain html, use htmlentities() to escape it prior to output. Remember many XSS attacks utilise malformed html, take advantage of buggy html rendering, or try and trick filtering systems with various encoding tactics.
For the database use the native database escaping functions (if they exist); mysql_real_escape_string, pg_escape_string, pg_escape_bytea, etc. One point to remember for these, is that you should read up on how magic_quotes (a PHP input filter that escapes input - not all that popular since it interferes with later escaping) effects these.
You should use sessions to store information (not absolutely required in a cookie) on the server which is needed from page request to page request.
Re: Is this Secure?
Posted: Sun Nov 27, 2005 9:45 am
by shiflett
Dark[NSF] wrote:Code: Select all
if (isset($_COOKIE['tj_email']) && isset($_COOKIE['tj_password']))
{
$input_email = $_COOKIE['tj_email'];
$input_password = $_COOKIE['tj_password'];
}
Authenticating users on every single request is a bad idea for a number of reasons. The top two that come to mind are:
1. You're exposing the authentication credentials in every request, likely even those for embedded resources.
2. You're making replay attacks particularly easy.
Encrypting these cookies does not eliminate both of these concerns. The approach itself is flawed, and it's always best to address the root cause of a problem rather than a symptom.
Dark[NSF] wrote:Code: Select all
header("Location: index.php?message=no_permissions");
The Location header requires an absolute URL. Most browsers are friendly and try to deduce what you mean, but IE has been known to have trouble properly handling cookies and malformed headers.
Hope that helps.
Posted: Sun Nov 27, 2005 10:43 am
by josh
shiftlett, your points about authentication are completely valid, but I would like to elaborate by saying by not checking some kind of credential on every page request you open up your users in a way.
Let's look at this scenario
User A log into his account and sets his password to "password"
User B asks User A for his password (and for whatever reason he gives it to him)
User A realizes what he has done and changes his password
but too late, User B is already logged in
Now of course you can prevent User B from completely taking over the account (by prompting him for his "old password" on the change password page), but now User B is free to trash the account in other aspects.
My proposed solution to this concern would be a randomized token hashed with the user's [hashed] password. Store the hashed password and whatever random string was used to generate the token in the session. You regenerate the token on each page request if you like. Now when the user changes his password the token is no longer valid.
I would personally never sacrifice an extra layer of security because of one call to the database, and I'm not clear on how this would be any more prone to a replay attack? Some kind of credential has to be sent on each page request regardless of the method being used (unless for some reason you are trusting the user's IP). At least this way a replay must be done before the user makes a new page view or logs out. With a standard session a replay can be done anytime before the session expires, right?
I'm not preaching here, I'm just trying to learn why this solution wouldn't work. Or would it?
Posted: Sun Nov 27, 2005 3:16 pm
by Ambush Commander
Well, if you just store all the (totally randomly generated) tokens in a db and then invalidate them all when a user changes their password...