security feature giving me an errorr - sessions mysql

[sophielois]

Hi,
Im trying to put togther an include file that im putting at the top of each "restricted to registered users page" to act as a security check

heres the code im playing with

<?
session_start();
header("Cache-control: private"); 

// 1st security check
$sql = mysql_query("SELECT * FROM candidates WHERE username ='$_SESSION['cka']' AND password ='$_SESSION['ckb']'");
$sec_chk = mysql_num_rows($sql);

if($sec_chk > 1){
header ("Location: ../index.php");
exit();
} else {}
?>

its returning the following

Parse error: parse error, unexpected T_ENCAPSED_AND_WHITESPACE, expecting T_STRING or T_VARIABLE or T_NUM_STRING in /home/n/v/nvqwandp/public_html/nvq/index.php on line 6

I've registered the sessions

$_SESSION['chk1'] = $password;
$_SESSION['chk2'] = $username;

I think the parse error is to do with '$_SESSION['cka'] and '$_SESSION['ckb'] but im unsure as to how to fix it.

Is this a good enough security check (providing it works) ?

Thanks
Soph

 

[DRJ478]

$sql = mysql_query("SELECT * FROM candidates WHERE username ='$_SESSION[[COLOR=red]'[/color]cka[COLOR=red]'[/color]]' AND password ='$_SESSION[[COLOR=red]'[/color]ckb[COLOR=red]'[/color]]'");

The single quotes render the expression unusable since they interrupt the string context. Concatenation will solve the problem:

$sql = mysql_query("SELECT * FROM candidates WHERE username ='".$_SESSION['cka']."' AND password ='".$_SESSION['ckb']."'");

You could also use sprintf with a format string to achieve what you want:

$format = "SELECT * FROM candidates WHERE username ='%s' AND password ='%s'");
$sql = sprintf($format,$_SESSION['cka'],$_SESSION['ckb']);

 

[sophielois]

ok cool, i like it. Thank you

DRJ478 could you give me your thoughts on this as a security device.

Will it do??? or should i add more to it?? Is it especially vulnerable??

I was just checking to see if the user had a couple of sessions registered and if they matched a certain variable. But i felt this would be more secure way of doing it.

I would be interested in your thoughts.

thank you for your post

Soph

 

[sophielois]

I was just checking to see if the user had a couple of sessions registered and if they matched a certain variable. But i felt this would be more secure way of doing it.

should read

Previously I was just checking to see if the user had a couple of sessions registered and if they matched a certain variable. But i felt this would be more secure way of doing it.

 

[DRJ478]

How secure does it have to be?

I see no difference between just relying on the session variables established at login and the query you show. The query uses the session vars, so it's not substantially different from just using the session data.

The only thing that could happen to 'break' in would be session hijacking or that the session was created by session fixation.

 

[sophielois]

oops another error

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /home/n/v/nvqwandp/public_html/nvq/index.php on line 8

???

 

[DRJ478]

Look at the faq434-3850 - it tells about the error checking one should have with MySQL queries etc.
Currently your code has no error checking, which produces a warning when the query fails - and we have no indication yet why it failed.

 

[sophielois]

How secure does it have to be?

there is no credit cards or information of that sort just privacy issues i suppose.

So checking that the username and password registered in the session variables match that found in the database should serve secure enough?

 

[DRJ478]

I would say so.
How often do you need to check?
I'd recommend to just do it at logon - because what would change after that?

Pseudo-op-code:
1. Collect username and password
2. Compare to stored record
3. If fail: retry/deny access
If pass: set session variable that user is authorized
4. On each restricted page look for authorized user session var (no database check needed)

Nobody can 'manipulate' session variables from the client side. The only thing the client has is the cookie with the session ID. Session variables are stored server side.
The verification of already established credentials is almost wasteful.

 

[sophielois]

hi DRJ478

ok so i could set
$_SESSION['authorized'] = "somerandomstring";

when a user successfully logs in and then do something like this on each page

<?
session_start();
header("Cache-control: private"); 

$sec_check="somerandomstring";
if ($_SESSION['authorized']!=$$sec_check){
header ("Location: ../index.php");
exit();
} else {}
?>

Soph

 

[DRJ478]

The mere presence of the 'authorized' session variable should suffice - no matter what is contained.
Unless there is some code injection possible the client has no way to generate that session var.
If you set $_SESSION['authorized_user'] = 'whoever' and check for isset($_SESSION['authorized_user']) you should be fine.