please help me make registration secure

[tbobker]

I am building a website that has user registration. I have almost finished the form with javascript checking user input...etc.

However, im not sure how i can properly check user input on the server side using php. At present i have a class called security, eventually i will have all data passed through this class.

The main registration page that takes the data is below:

<?php 
	require("classes/database.php");
	$database_fun = new database();
	$check_Fields = new security();

include("include/header.php");

	if(!isset($_POST['submit'])){
		echo "hacker attempt";
		return;
	}else{		
		$key = md5(rand());
                    // run $_POST through security class 
		$regPosts = $check_Fields->postvars($_POST);
		if($database_fun->query("insert into user values(
									'','".$regPosts['artType']."','".$regPosts['username']."','".$regPosts['pass1']."','".$regPosts['email']."','".$regPosts['country']."',
									'".$regPosts['city']."','".date('H:i, jS F')."','".date('H:i, jS F')."','".$regPosts['website']."','".$regPosts['biography']."',
									'".$key."')")){										
				// eventually an email will be created and sent to the user
				echo "Your details have been emailed to you please activate your account using the link in your email<br />
				      www.theglobalarts.com/profile.php?activate=$key";
		}else{
			die("The system was unable to create a new user");
			exit;
		}

	}
include("include/footer.php"); ?>	
?>

The postvars function in the security class just "addslashes()" to the $_POST values like the below:

	function postvars($postFields){
		foreach($postFields as $name=>$value){
			$posts[$name] = addslashes($value);
			$counter ++;
		}
		return $posts;
	}

The above function is ready to check all the values from the $_POST and throw an error if something is not correct and validate all the users input - im just not sure what else to check for and how? Any pointers really appreciated.

[rulian]

You have a right idea, definately always filter users inputs.
even though addslashes helps, definately check out mysql_real_escape_string and prepared statements using sprintf()
it's a good standard in security, read up on sql injection techniques

[tbobker]

ok thanks for the help.

[laserlight]

In this case it appears that you are using some database wrapper. Instead of blindly using mysql_real_escape_string(), use the escaping mechanism provided by the database wrapper. If you wrote the database wrapper yourself, provide such an escaping mechanism (e.g., a string_escape() function) and implement it with mysql_real_escape_string().

[tbobker]

thanks for the advice but im not quite sure what you mean by "wrapper". I am trying to write a class with functions that check all data. One function to check the URL one to check the form values (the one concerned above).

would this be valid:

mysql_real_escape_string(addslashes($_POST['value']));

is the above what you mean?

[laserlight]

thanks for the advice but im not quite sure what you mean by "wrapper".

I am talking about the class that had an object instantiated here:

$database_fun = new database();

I am trying to write a class with functions that check all data.

You could write a class, but it may not be necessary if the functions that check the data are specific to this part of the script.

would this be valid:

No, that is incorrect. Just use mysql_real_escape_string().

is the above what you mean?

No, what I mean is:

class database {
    // ...

public function escape_string($str) {
    return mysql_real_escape_string($str);
}

// ...
}

Then in your current script you will use things like:

$database_fun->escape_string($regPosts['artType']);

[tbobker]

so to validate the user input data from a form using mysql_real_escape_string() is enough?

[laserlight]

It is a minimum for string input if you are using the MySQL API.

[rulian]

but using what laserlight described, if you ever switch to another database like posgresql, you simply change that one line in your class rather then the dozens or hundreds of mysql_real_escape_string you would find in your code, that makes your code portable.

[bradgrafelman]

Also note other escaping methods, such as using [man]intval/man or casting the data to an integer when you expect such an input (since I don't use quotes around numeric values in my query, if I didn't do this then a string input would break my queries).