Jump to content

This Php Scripts Have A Secure Problem?


phpnoob
 Share

Recommended Posts

Hello.I just want to know, my full website have a secure problem somewhere, or can be hackit?MYSQL tableColumn "id" + "nick" + "pass" + "email" + "userid" this for forgetpass, confirmcode, + "ip" + "cookie"<--unique + "session"<--unique + "user"+"mod"+"admin"Reg.php

$nick=mysql_real_escape_string($_POST['nick']);$email=mysql_real_escape_string($_POST['email']);$pass1=mysql_real_escape_string($_POST['pass1']);$pass2=mysql_real_escape_string($_POST['pass2']);if ($_POST['submit']){	if (!empty($nick) && !empty($email) && !empty($pass1) && !empty($pass2) )	{		if (strlen($nick) >=5)		{			if(ereg('[a-zA-Z0-9\-\_áéíóöüóűÁÉÍÓÖÜŐŰ]+$', $nick))			{				if(eregi("^[A-Z0-9._%-]+@[A-Z0-9.-]+\.[A-Z]{2,4}$", $email))				{					if(mysql_num_rows(mysql_query("SELECT nick FROM users WHERE nick='".$nick."' "))==0)					{						if(mysql_num_rows(mysql_query("SELECT email FROM users WHERE email='".$email."'"))==0)						{							if ($pass1==$pass2)							{								if (strlen($pass1) >= 5 && strlen($pass1) <= 25)								{								$pass=md5($pass1);$date=date("Y-m-d H:i:s");$random=md5(rand(1,99999999999));$ip=$_SERVER['REMOTE_ADDR'];								$sql="INSERT INTO users (nick,pass,email,cookie,session,ip,regdate) VALUES('$nick','$pass','$email','$random','$random','$ip','$date')";									if (mysql_query($sql))									{									echo 'Success Registration.'; echo '<a href="index.php">next</a>';									}									else									{									echo 'Database error';									}								}								else								{								echo "The length of the password are between 5-25!";								}							}							else							{							echo "The two password are not the same";							}						}						else						{						echo 'The mail address you entered are taken'; 						}					}					else					{					echo 'The user name you entered are taken';					}				}				else				{				echo "Wrong Email address.";				}			}			else			{			echo "The nick name char ... ";			}		}		else		{		echo ' Nicknname: the minimum length are 5 .';		}	}	else	{	echo "You are know fill in all input";	}}

This character are hungary character "áéíóöüóűÁÉÍÓÖÜŐŰ"Login.php "The Form have 1 checkbox for cookie"

if($_POST['login']){	if(!empty($_POST['name']) && !empty($_POST['pass']))	{	$random=md5(rand(1,99999999999));$ip=$_SERVER['REMOTE_ADDR'];$nick=mysql_real_escape_string($_POST['nick']);$pass=mysql_real_escape_string(md5($_POST['pass']));	$sql="SELECT * FROM users WHERE nick='".$nick."' and pass='".$pass."' ";$query=mysql_query($sql);		if($loginrow=mysql_fetch_array($query))		{		$rememberme=$_POST['rememberme'];			if ($rememberme=="on")			{			$cookieupdate="UPDATE users SET cookie='".$random."', ip='".$ip."' WHERE nick='".$nick."' and pass='".$pass."'";				if(mysql_query($cookieupdate))				{				setcookie("CookieID", $random, time()+604800);$_SESSION['nick']=$nick;header("Location: ".$_SERVER['PHP_SELF']);				}				else				{				echo 'Database error, press F5 ';/* The cookie column in MYSQL are unique, you know why */				}			}			elseif($rememberme=="")			{			$sessionupdate="UPDATE users SET session='".$random."', ip='".$ip."' WHERE nick='".$nick."' and pass='".$pass."'";				if(mysql_query($sessionupdate))				{				$_SESSION['nick']=$nick;$_SESSION['sesID']=$random;header("Location: ".$_SERVER['PHP_SELF']);				}				else				{				echo 'Database error, press F5'; /* The session column in MYSQL are unique, you know why */				}			}		}		else		{		echo '<br>Wrong nickname/password!';		}	}	else	{	echo 'You are not fill in all input.';	}}

checkcookieandsession.php

function check(){	if(empty($_SESSION['sesID']))	{		if(!empty($_COOKIE['CookieID']))		{		$cookie=mysql_real_escape_string($_COOKIE['CookieID']);$ipcheck=$_SERVER['REMOTE_ADDR'];$cookiequery=mysql_query("SELECT * From users WHERE cookie='$cookie'");			if($cookiecheck=mysql_fetch_assoc($cookiequery))			{				if($cookiecheck['ip']==$ipcheck)				{				$_SESSION['nick']=$cookiecheck['nick'];return  "cookiehave";				}				else				{				setcookie("CookieID","", time()-3600);header("Location: ".$_SERVER['PHP_SELF']);				}			}		}		else		{		return "Cookienothave";		}	}	else	{	return "sessionhave";	}}

mod and admincheck php

if(check()==cookiehave or check()==sessionhave){$checkstatus=mysql_query("SELECT * FROM users where nick='".$_SESSION['nick']."' ");	if ($statusrow = mysql_fetch_assoc($checkstatus))	{	$user=$statusrow['user'];$mod=$statusrow['mod'];	$admin=$statusrow['admin'];	}}

Password.php

if(check()==cookievan or check()==sessionvan){ /*Online Section*/	if ($_POST['submit'])	{		if (!empty($_POST['oldpass']) &&! empty($_POST['newpass1']) && !empty($_POST['newpass2']))		{		$oldpass=mysql_real_escape_string(md5($_POST['oldpass']));$newpass1=mysql_real_escape_string($_POST['newpass1']);$newpass2=mysql_real_escape_string($_POST['newpass2']);			if ($newpass1 == $newpass2)			{				if (strlen($newpass1)>=5&& strlen($newpass1)<=25)				{				$nick=$_SESSION['nick'];$passquery=mysql_query("SELECT * FROM users WHERE nick='".$nick."' and pass='$oldpass'");					if($passrow=mysql_fetch_assoc($passquery))					{					$newpass=md5($newpass1);$update="UPDATE users SET pass='$newpass' WHERE nick='".$row['nick']."' and id='".$row['id']."' ";						if (mysql_query($update))						{						echo 'The password succesfully updated <meta http-equiv="refresh" content="1;url=index.php">';						}						else						{						echo "DatabaseError";						}					}					else					{					echo 'Wrong Password.';					}				}				else				{				echo "The password length are must be between 5-20!";				}			}			else			{			echo "The 2 password not same";			}		}		else		{		echo 'You must fill in all input';		}	}}else{/*Offline Section*/	if ($_POST['submit'])	{		if (!empty($_POST['confirmcode']) && !empty($_POST['pass1']) && !empty($_POST['pass2']) && !empty($_POST['email']))		{		$email=mysql_real_escape_string($_POST['email']);$confirmcode=mysql_real_escape_string($_POST['confirmcode']);$pass1=mysql_real_escape_string($_POST['pass1']);$pass2=mysql_real_escape_string($_POST['pass2']);			if ($pass1 == $pass2)			{				if (strlen($pass1)>=5&& strlen($pass1)<=25)				{					if (mysql_num_rows(mysql_query("SELECT * FROM users WHERE userid='$confirmcode' and email='$email'")) == 1)					{					$pass=md5($pass1);$passupdate="UPDATE users SET pass='$pass', userid='' WHERE email='$email' and userid='$confirmcode'";						if (mysql_query($passupdate))						{						echo "The password succesfully updated.";						}						else						{						echo "Database Error";						}					}					else					{					echo "Wrong confirmcode or mail address. if the mail address is good than see again your mailcode, on your forgetpass mail";					}				}				else				{				echo "The password length are must be between 5-20!";				}			}			else			{			echo "The 2 password not same";			}		}		else		{		echo 'You must fill in all input';		}	}}

Website PHPs

if ($mod==1){Moderator section}if ($admin==1){Admin section}if($user==1){user section}else{banner}if(check()==cookiehave or check()==sessionhave){loged user section}

Edited by phpnoob
Link to comment
Share on other sites

Please indent your code... it's next to impossible to see anything.Also keep in mind that there's never 100% security. Whatever we say, any app that accepts user input can always potentially be hacked. You can (and should) minimize that chance of course.

Link to comment
Share on other sites

I'm not sure as for security, but for stability and extensibility's sake, I suggest you do your mysql_real_escape_string() when you construct the query, not before. Otherwise, $nick becomes useless outside of the query. What if you want to display the username for example? If it contains special and/or unanticipated characters (for a MySQL query that is...), do you really want to output the name with "\" in front of such characters? Doing so would look ugly.On the other hand, to prevent XSS attacks, you'd want to pass $nick over htmlspecialchars(). Again, you'll actually be escaping the escaped MySQL string, not the real string.Also, doing

mysql_num_rows(mysql_query(

is not exactly a good practice, as if mysql_query() was to return FALSE on failure (which can happen if the SQL connection is suddenly lost in the middle of the script for example or on a successful SQL injection), mysql_num_rows() emits an error, which could potentially be used by attackers to diagnose how your code operates.I'd instead suggest you store the result of mysql_query(), and if it's not false, do the rows check.

Link to comment
Share on other sites

I'm not sure as for security, but for stability and extensibility's sake, I suggest you do your mysql_real_escape_string() when you construct the query, not before. Otherwise, $nick becomes useless outside of the query. What if you want to display the username for example? If it contains special and/or unanticipated characters (for a MySQL query that is...), do you really want to output the name with "\" in front of such characters? Doing so would look ugly.On the other hand, to prevent XSS attacks, you'd want to pass $nick over htmlspecialchars(). Again, you'll actually be escaping the escaped MySQL string, not the real string.Also, doing
		if($loginrowa=$query)			{				if(mysql_num_rows($loginrowa)==1)				{	

Some otherAll php are captured in function, only three php is notIndex.phpadmin.phpuser.phpcontent:$something=empty($_GET['cat']) ? 'board' : $_GET['cat'];$num=$_GET['n'];if ($something==login){include "include/login.php"; echo login();}login.php "function login(){php codes}"all php looks like this.This is bad?

Link to comment
Share on other sites

if(ereg('[a-zA-Z0-9\-\_áéíóöüóűÁÉÍÓÖÜŐŰ]+$', $nick))nick character limited to this characters, so no need htmlspecialchars(), i think.
Have you tested that? What happens if you use this for the nick:<script type="text/javascript" src="http://malware.com/bad.js"></script>Joe
1 the maxlength of the login and pass input are 25
Doesn't matter, an attacker can send any data they want. They don't have to use your form.
if ($something==login)
Unless login is a constant, it should be quoted. It would probably help you to turn on all errors, add this to the top of each of your pages:ini_set('display_errors', 1);error_reporting(E_ALL);
Link to comment
Share on other sites

Do you use $something and $num everywhere? If so, then I guess using the content structure you have is fine.

First check the reg.phpif(ereg('[a-zA-Z0-9\-\_áéíóöüóűÁÉÍÓÖÜŐŰ]+$', $nick))nick character limited to this characters, so no need htmlspecialchars(), i think.
True, but only if you output $nick within that condition... but in a registration process, you'd usually want to output a name when it's invalid, and therefore, it may not match that regexp.
Second: login process1 the maxlength of the login and pass input are 252 checking, if the query search is not 1, than end the process, if the process hacked that third step3 cookie or session column update, if the search get 2 or more, than no update, because the cookie and session column are uniqued if this process hacked than its over.
The stuff you need to ask yourself consistently is not what you do when you succeed, but what do you do when you fail. Upon any failure, you need to assume everything as being invalid, and sanitize it before sending it to its destination (with the browser also being a destination).
Link to comment
Share on other sites

Do you use $something and $num everywhere? If so, then I guess using the content structure you have is fine.True, but only if you output $nick within that condition... but in a registration process, you'd usually want to output a name when it's invalid, and therefore, it may not match that regexp.The stuff you need to ask yourself consistently is not what you do when you succeed, but what do you do when you fail. Upon any failure, you need to assume everything as being invalid, and sanitize it before sending it to its destination (with the browser also being a destination).
now i'm understand what you trying to tell meThan i modifying the failure side. to Header("Location: ".$_SERVER['REQUEST_URI']); or PHP_SELFor if this is not a idea, tell me what can i do Edited by phpnoob
Link to comment
Share on other sites

I can sense a reluctance to test anything, so let me just tell you.

if(ereg('[a-zA-Z0-9\-\_áéíóöüóűÁÉÍÓÖÜŐŰ]+$', $nick))nick character limited to this characters, so no need htmlspecialchars(), i think.Have you tested that? What happens if you use this for the nick:<script type="text/javascript" src="http://malware.com/bad.js"></script>Joe
That string will pass straight through that regex, it will not flag it as invalid. Why do you think that is? More importantly, why don't people want to test their applications?
Link to comment
Share on other sites

Redirecting to another page isn't going to preserve the data.Legitimate users that made errors by mistake will want to see whatever they have typed, and you're likely to annoy them if they have to retype everything.Oh, and I can see what justsomeguy means with the regex... hint: it's a lack of a feature that PHP6 will have, but for PHP5, you'll need to workaround.

Link to comment
Share on other sites

Sorry i don't see your post justsomeguyLogin only 1 place have, in the main pageIndex.phpcontent$getpage=empty($_GET['listazas']) ? 'news' : $_GET['listazas']; listazas=query for my languageAdmin.phpContent$getadmin=empty($_GET['cat']) ? 'board' : $_GET['cat']; $num=$_GET['n'];<-- this is for id searching "del and update", and this php not have login, only cookie + status checkingUser.phpContent$cat=$_GET['cat'];Than what can i do for ereg?And what can i do this one?ini_set('display_errors', 1);error_reporting(E_ALL);

Link to comment
Share on other sites

Your regular expression is checking if any of the characters listed come before the end of the string. You're not checking the beginning of the string, only the end. That's why you can insert any code you want, as long as the last few characters match your pattern. This is why you need to test things. Don't just copy a regex and assume it works for what you want, try to enter data that should break it and see what happens.Also, don't use ereg any more, use preg instead. ereg is being removed from PHP.http://www.php.net/manual/en/function.preg-match.phpRead this to see how to define the start of the string:http://www.php.net/manual/en/regexp.refere...circudollar.php

Link to comment
Share on other sites

Damn... guilty of missing the missing "^". I though the problem is the lack of Unicode support in PHP, which could have lead to the addition of a few bytes being legal, among which are the ones required to make an XSS attack.Still, the whole thing is an illustration of what I had in mind - don't rely on your input validation working - sanitize stuff appropriately afterwards, just in case.

Link to comment
Share on other sites

and when it comes to this kind of form validation stuff, try letting your friends hack around with it and see if they can break it. What you might know in your mind as "acceptable" values, other people won't already have any knowledge of, and such you'll come across all sorts of unlikely submissions, names, values, etc. A great point was made in that it's not what succeeds that's the most important, it's what is supposed to fail and how you handle it.

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...