Php Securty

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Php Securty

Post by doggy »

I am trying to make a pritty secury login for a gallery. I just wane know if some one could help me out in a way i could secure it more. At the moment i have

Code: Select all

<?php
function m_user($usr, $pwd) {
	$session_time;
	dbi();
	$sql = "SELECT * FROM users WHERE username = '$usr' AND password = '$pwd'";
	$result = mysql_query($sql);
	if (mysql_num_rows($result) == 1) {
		$sql1 = "SELECT * FROM users WHERE username = '$usr' AND password = '$pwd'";
		$result1 = mysql_query($sql1);
		$row = mysql_fetch_array($result1);
		$userid = $row['uid'];
		$_SESSION['userid'] == $userid;
		$sql2 = "INSERT INTO active_users (sid, uid, username, usertime) VALUES (NULL,'$userid','$usr','$session_time')";
		$result2 = mysql_query($sql2);
	}	
}
?>
Setting a Session called userid to use the user id where every i want information from the user and setting a row in active_user to check if the user is still active and if not that row will be removed and if the user is in the 20 min session time that row will be updated.

How secure is this script and is there any thing els i can add to make securety even better.
lostboy
Forum Contributor
Posts: 329
Joined: Mon Dec 30, 2002 8:12 pm
Location: toronto,canada

Post by lostboy »

open like a sieve...no data validation, no checking for sql injection attacks...here's simple test

under password type in ' or 1=1; '...if i were mean it would be '; drop database
User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Post by doggy »

I don`t anderstand what you mean with - Here is a simple test under password type in or 1=1;
User avatar
launchcode
Forum Contributor
Posts: 401
Joined: Tue May 11, 2004 7:32 pm
Location: UK
Contact:

Post by launchcode »

Do what he said - on your login form enter the text "or 1=1" as the password and see what happens.
User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Post by doggy »

or 1=1 does nothing didn`t want to log me in
User avatar
launchcode
Forum Contributor
Posts: 401
Joined: Tue May 11, 2004 7:32 pm
Location: UK
Contact:

Post by launchcode »

Copy the exact text he used: ' or 1=1; ' (include the quotes).
Unless you have magic_quotes enabled without knowing it, the extra quote mark in the text he gave you would basically screw your SQL query up totally, making it look like this:

SELECT * FROM users WHERE username = '?' AND password = ' ' or 1=1; ''

In other words - you'd always be able to login (because 1 is always equal to 1).
User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Post by doggy »

' or 1=1; ' in the password field does not work , I don`t know why . LOl or really i am happy it doesn`t work i entered no username and ' or 1=1; ' and even a username and ' or 1=1; ' and still doesn`t work even ' or 1=1; ' and ' or 1=1; ' does not work , sorry
User avatar
launchcode
Forum Contributor
Posts: 401
Joined: Tue May 11, 2004 7:32 pm
Location: UK
Contact:

Post by launchcode »

Yeah, you've got magic quotes enabled then (and didn't even know it.. heh).

Basically all of his original comments still apply though - there isn't a single piece of data validation going on, you don't check the length of the values, if they are strings or numbers, if they contain HTML or SQL code, heck if they even exist or not. They just get dumped directly into a SQL query. See where we're coming from now?
User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Post by doggy »

Yea i am , what i will do is i will check the lenth of the input and then check if it is set and so on. Thanks for the help we paste the new code again and check what you guys think about it. Thanks
User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Post by doggy »

How is this , New function added c_value will check if some values is in or not in the vlaues.

Code: Select all

<?php
function c_value($value) {
	if (strpos($value, "<html>")) {
		return true;
	}
	if (strpos($value, "1=1")) {
		return true;
	}
	if (strpos($value, ";")) {
		return true;
	}
	if (strpos($value, "@")) {
		return true;
	}
	if (strpos($value, "$")) {
		return true;
	}
	if (strpos($value, "%")) {
		return true;
	}
	return false;
}

function m_user($usr, $pwd) {
	if ((strlen($usr) < 5) && (strlen($pws) < 5)) {
		if ((c_value($usr) == false) && (c_value($pws) == false)) { 
			$session_time = m_s_time();
			dbi();
			$sql = "SELECT * FROM users WHERE username = '$usr' AND password = '$pwd'";
			$result = mysql_query($sql);
			if (mysql_num_rows($result) == 1) {
				$sql3 = "SELECT * FROM users WHERE username = '$usr' AND password = '$pwd'";
				$result3 = mysql_query($sql3);
				$row = mysql_fetch_array($result3);
				$userid = $row['uid'];
				$_SESSION['userid'] = $userid;
				$sql2 = "INSERT INTO active_users (sid, uid, username, usertime) VALUES (NULL,'$userid','$usr','$session_time')";
				$result2 = mysql_query($sql2);
				Header("Location: index.php?f=tg");
			} else {
				Header("Location: index.php?f=lin");
			}
		} else {
			Header("Location: index.php?f=lin");
		}
	} else {
		Header("Location: index.php?f=lin");
	}
}
?>
User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Post by doggy »

So no one have any thing els to say about my securety Q ....
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Code: Select all

$sql3 = "SELECT * FROM users WHERE username = '$usr' AND password = '$pwd'"; 
            $result3 = mysql_query($sql3);
looks a bit redundant.

unless index.php does some verifcation on $_GET['f'], you could have a hole there too.
lostboy
Forum Contributor
Posts: 329
Joined: Mon Dec 30, 2002 8:12 pm
Location: toronto,canada

Post by lostboy »

Code: Select all

//simple regex valdation to allow only numbers and letters
//does depend on whatever password validation scheme you are using
if (!eregi("[[]]",$pasword){
   echo "Alphabetical and numbers only";
}
...
//redundant code only need to do one query
$sql = "SELECT * FROM users WHERE username = '$usr' AND password = '$pwd'"; 
         $result = mysql_query($sql); 
         if (mysql_num_rows($result) == 1) { 
            $row = mysql_fetch_array($result); 
            $userid = $row['uid']; 
            $_SESSION['userid'] = $userid; 
            $sql2 = "INSERT INTO active_users (sid, uid, username, usertime) VALUES (NULL,'$userid','$usr','$session_time')"; 
            $result2 = mysql_query($sql2); 
            Header("Location: index.php?f=tg"); 
         } else {
Last edited by lostboy on Fri May 21, 2004 12:49 pm, edited 1 time in total.
User avatar
doggy
Forum Commoner
Posts: 80
Joined: Tue Dec 09, 2003 5:01 am
Location: South Africa

Post by doggy »

feyd wrote:

Code: Select all

$sql3 = "SELECT * FROM users WHERE username = '$usr' AND password = '$pwd'"; 
            $result3 = mysql_query($sql3);
looks a bit redundant.

unless index.php does some verifcation on $_GET['f'], you could have a hole there too.
I didn`t do the check if user on the index.php page yet but just trying to get the making the user active working with out any holes.
ghost007
Forum Commoner
Posts: 49
Joined: Sat Nov 22, 2003 10:10 am

Post by ghost007 »

here is a funtion that you can use to check all form inputs:

viewtopic.php?t=20193&highlight=

Personaly I also think that most security holes comes from the way you identify users on your pages.

hope this helps

siech
Post Reply