Need to stop users inputting malicious code via user form

Questions about the MySQL, PostgreSQL, and most other databases, as well as using it with PHP can be asked here.

Moderator: General Moderators

Post Reply
liiam
Forum Newbie
Posts: 3
Joined: Mon Aug 13, 2007 2:04 pm

Need to stop users inputting malicious code via user form

Post by liiam »

hello. I have a userform that allows you to sign up with a username and password. make a profile and later, log in and edit that same profile. all using forms.

background on me is that im learning as i go, trying to bring this site idea i have to life. Im using forums, tutorials, sample code and kind people to help me do this.

Anyways someone ponted out to me that my current form allows for users to input javascript which poses a security risk seens as my login system uses sessions.

what that same someone wont tell me is how to fix this. so im wondering if anyone here would mind helping me?

here is the sign up form. Its 2 pages

Code: Select all

signup.php 

<?


include "include/z_db.php";// database connection details stored here

?>
<html>
<head>
<title>Sign Up</title>
</head>

<body bgcolor="#ffffff" text="#000000" link="#0000ff" vlink="#800080" alink="#ff0000">

<table border='0' width='50%' cellspacing='0' cellpadding='0' align=center><form name=form1 method=post action=signupck.php onsubmit='return validate(this)'><input type=hidden name=todo value=post>

<tr bgcolor='#f1f1f1'><td align=center colspan=2><font face='Verdana' size='2' ><b>Signup</b></td></tr>
<tr ><td >&nbsp;<font face='Verdana' size='2' >Username:</td><td ><font face='Verdana' size='2'><input type=text name=username></td></tr>

<tr bgcolor='#f1f1f1'><td >&nbsp;<font face='Verdana' size='2' >Password:</td><td ><font face='Verdana' size='2'><input type=password name=password></td></tr>
<tr ><td >&nbsp;<font face='Verdana' size='2' >Re-enter Password:</td><td ><font face='Verdana' size='2'><input type=password name=password2></td></tr>

<tr bgcolor='#f1f1f1'><td ><font face='Verdana' size='2' >&nbsp;Email:</td><td  ><input type=text name=email></td></tr>
<tr ><td >&nbsp;<font face='Verdana' size='2' >Band Name:</td><td ><font face='Verdana' size='2'><input type=text name=bandname></td></tr>
<tr ><td >&nbsp;<font face='Verdana' size='2' >Band Bio:</td><td ><font face='Verdana' size='2'><input type=text name=bio></td></tr>
<tr ><td >&nbsp;<font face='Verdana' size='2' >Band Website:</td><td ><font face='Verdana' size='2'><input type=text name=site></td></tr>
<tr ><td >&nbsp;<font face='Verdana' size='2' >URL To Band photo:</td><td ><font face='Verdana' size='2'><input type=text name=image></td></tr>

<tr bgcolor='#f1f1f1'><td align=center colspan=2><input type=submit value=Signup></td></tr>
</table>

</body>
</html>
and

Code: Select all

signupck.php

<?
include "include/z_db.php";// database connection details stored here

while (list ($key,$val) = each ($_POST)) {
$$key = $val;
}

?>
<!doctype html public "-//w3c//dtd html 3.2//en">

<html>

<head>
<title>Sign Up</title>

</head>

<body bgcolor="#ffffff" text="#000000" link="#0000ff" vlink="#800080" alink="#ff0000">
<?
if(isset($todo) and $todo=="post"){

$status = "OK";
$msg="";

// if userid is less than 3 char then status is not ok
if(!isset($username) or strlen($username) <3){
$msg=$msg."Username should be =3 or more than 3 char length<BR>";
$status= "NOTOK";}					

if(mysql_num_rows(mysql_query("SELECT username FROM info WHERE username = '$username'"))){
$msg=$msg."Userid already exists. Please try another one<BR>";
$status= "NOTOK";}					


if ( strlen($password) < 3 ){
$msg=$msg."Password must be more than 3 char legth<BR>";
$status= "NOTOK";}					

if ( $password <> $password2 ){
$msg=$msg."Both passwords are not matching<BR>";
$status= "NOTOK";}					


if($status<>"OK"){ 
echo "<font face='Verdana' size='2' color=red>$msg</font><br><input type='button' value='Retry' onClick='history.go(-1)'>";
}else{ // if all validations are passed.
$query=mysql_query("insert into info(username,password,email,bandname,bio,site,image) values('$username','$password','$email','$bandname','$bio','$site','$image')");
echo "<font face='Verdana' size='2' color=green>Welcome, You have successfully signed up<br><br><a href=login.php>Click here to login</a><br></font>";
}
}
?>

</body>
</html>
apologies if ive violated any forum rules with this request and format of my post but i think ive behaved correctly.

Thanks for reading
Liam


User avatar
onion2k
Jedi Mod
Posts: 5263
Joined: Tue Dec 21, 2004 5:03 pm
Location: usrlab.com

Post by onion2k »

HTMLPurifier is the best tool for that job.
User avatar
kaszu
Forum Regular
Posts: 749
Joined: Wed Jul 19, 2006 7:29 am

Post by kaszu »

Few things i noticed:
1)

Code: Select all

while (list ($key,$val) = each ($_POST)) {
$$key = $val;
}
This allows users to override your defined variables.
2) Also your code is vulnerable to SQL injections.
To fix both:

Code: Select all

$input_data = Array();
while (list ($key,$val) = each ($_POST)) {
$input_data[$key] = mysql_real_escape_string($val);
}
and then instead of $username use $input_data['username']

Maybe I wasn't looking hard enough, but in this code i didn't saw problems with Javascript unless you echo later username without escaping it, which should be done with htmlentities()
liiam
Forum Newbie
Posts: 3
Joined: Mon Aug 13, 2007 2:04 pm

Post by liiam »

I went through and replaced $username with $input_data['username'] as suggested.

I now get this error when i try to sign up:
Parse error: syntax error, unexpected T_ENCAPSED_AND_WHITESPACE, expecting T_STRING or T_VARIABLE or T_NUM_STRING in /home/www/ys.hgbforum.net/signupck.php on line 32
my signupck.php page looks like this:

Code: Select all

<?

include "include/z_db.php";// database connection details stored here

$input_data = Array();
while (list ($key,$val) = each ($_POST)) {
$input_data[$key] = mysql_real_escape_string($val);
}

?>
<!doctype html public "-//w3c//dtd html 3.2//en">

<html>

<head>
<title>Sign Up</title>

</head>

<body bgcolor="#ffffff" text="#000000" link="#0000ff" vlink="#800080" alink="#ff0000">
<?
if(isset($todo) and $todo=="post"){

$status = "OK";
$msg="";

// if userid is less than 3 char then status is not ok
if(!isset($input_data['username']) or strlen($input_data['username']) <3){
$msg=$msg."Username should be =3 or more than 3 char length<BR>";
$status= "NOTOK";}					

if(mysql_num_rows(mysql_query("SELECT username FROM info WHERE username = '$input_data['username']'"))){
$msg=$msg."Userid already exists. Please try another one<BR>";
$status= "NOTOK";}					


if ( strlen($password) < 3 ){
$msg=$msg."Password must be more than 3 char legth<BR>";
$status= "NOTOK";}					

if ( $password <> $password2 ){
$msg=$msg."Both passwords are not matching<BR>";
$status= "NOTOK";}					


if($status<>"OK"){ 
echo "<font face='Verdana' size='2' color=red>$msg</font><br><input type='button' value='Retry' onClick='history.go(-1)'>";
}else{ // if all validations are passed.
$query=mysql_query("insert into info(username,password,email,bandname,bio,site,image) values('$input_data['username']','$password','$email','$bandname','$bio','$site','$image')");
echo "<font face='Verdana' size='2' color=green>Welcome, You have successfully signed up<br><br><a href=login.php>Click here to login</a><br></font>";
}
}
?>


</body>

</html>
And yeah I will talk about how / where I echo back info once I get this boxed off. best to do one thing at a time isnt it, and I figure security is a priority with any code so this first.

Thank you for your speedy responses!
I went with the second one as it was so quick. I will go google "HTMLPurifier" now

nice place. nice people.
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

Do you understand the code you're using? Or is it just copy-pasted together?

Code: Select all

$query=mysql_query("insert into info(username,password,email,bandname,bio,site,image) values('$input_data['username']','$password','$email','$bandname','$bio','$site','$image')");
echo "<font face='Verdana' size='2' color=green>Welcome, You have successfully signed up<br><br><a href=login.php>Click here to login</a><br></font>";
(Btw, you went through the trouble of preparing lots of values for use in a mysql query.. and then you're only using the $input_data['username'] from it)
SidewinderX
Forum Contributor
Posts: 407
Joined: Fri Jul 16, 2004 9:04 pm
Location: NY

Post by SidewinderX »

Code: Select all

if  (mysql_num_rows(mysql_query("SELECT `username` FROM `info` WHERE `username` = '" . $input_data['username'] . "'") or die (mysql_error()))) {
liiam
Forum Newbie
Posts: 3
Joined: Mon Aug 13, 2007 2:04 pm

Post by liiam »

I understand though what Ive read. but bear in mind ive only read things im told are relevant to what I want to acheive, I havent looked at general php code as such.

in response to
(Btw, you went through the trouble of preparing lots of values for use in a mysql query.. and then you're only using the $input_data['username'] from it)
Maybe I should explain what happens when a user sumbits that sign up form. the data is added to the relevant fields within the table "info" and is output later after a search.

Code: Select all

<?php
//
// search.php
//
include("include/z_db.php");
$q=(isset($_GET['q']) ? $_GET['q'] : '');

if ($q == "") {
?>
<b>Search The Site</b><br>
<form action="search.php" method="get">
Look For:<input type="text" name="q" maxlength="25" /><br />
<input type="submit" value="GO!" />
</form>
<?php
}
{
  $result=mysql_query("SELECT * FROM info WHERE bandname LIKE '%$q%'") or die(mysql_error());
  }
  
$num_rows=mysql_num_rows($result);

  if ($num_rows == 0) {
?>
<p><b>Results</b><br>Nothing Im afraid!</p>
<?php
  }
  elseif ($num_rows == 1) {
  $row=mysql_fetch_array($result);
?>
<p><b>Results</b><br><a href="profile.php?id=<?php echo $row['bandname']; ?>"><?php echo $row['bandname']; ?></a></p>
<br /><br />

<?php
    }
?>
and then

Code: Select all

<?php
//
// profile.php
//
include("include/z_db.php");
$id=(isset($_GET['id']) ? $_GET['id'] : '1');
$result=mysql_query("SELECT * FROM info WHERE bandname='$id'") or die(mysql_error());
$row=mysql_fetch_array($result);
?>
<p><b><?php echo $row['bandname']; ?></b></p><br>
<p><?php echo $row['bio']; ?></p>
<p><a href=<?php echo $row['site']; ?>><?php echo $row['bandname']; ?>'s site</a></b></p>
<p><img src=<?php echo $row['image']; ?>></p>
Post Reply