views:

86

answers:

2

i have a PHP contact form that submits data, and an email...:

<?php 
$dbh=mysql_connect ("localhost", "username", "password") or die ('I cannot connect to the database because: ' . mysql_error());
mysql_select_db ("guest"); 

if (isset($_POST['submit'])) { 

if (!$_POST['name'] | !$_POST['email']) 
{
echo"<div class='error'>Error<br />Please provide your Name and Email Address so we may properly contact you.</div>";
}
else
{
$age = $_POST['age']; 
$name = $_POST['name'];
$gender = $_POST['gender'];
$email = $_POST['email'];
$phone = $_POST['phone'];
$comments = $_POST['comments'];

$query = "INSERT INTO contact_us (age,name,gender,email,phone,comments)
VALUES ('$age','$name','$gender','$email','$phone','$comments')";

mysql_query($query);

mysql_close();

$yoursite = "Mysite ";
$youremail = $email;

$subject = "Website Guest Contact Us Form";
$message = "$name would like you to contact them 
                            Contact PH:  $phone
Email:  $email
Age: $age
Gender: $gender
Comments:  $comments";

$email2 = "[email protected]";

mail($email2, $subject, $message, "From: $email");

echo"<div class='thankyou'>Thank you for contacting us,<br /> we will respond as soon as we can.</div>";

}
}
?>

The email is coming through fine, but the data is not storing the dbase... am i missing something? Its the same script as i use on another contact us page, only difference is instead of parsing the data on teh same page, i now send this data to a "thankyou.php" page... i tried changing $_POST to $_GET but that killed the page... what am i doing wrong?

+3  A: 

First of all, you must escape your data before injecting them in your SQL query.

This can be done using the mysql_real_escape_string function, like this :

$name = mysql_real_escape_string($_POST['name']);
// ... same for other fields that contain strings
$comments = mysql_real_escape_string($_POST['comments']);


This will ensure that quotes in your data are escaped, and don't mess with the ones that are arround the fields' data in the SQL query, first.

And, second, this will help you prevent SQL Injections.


Also, in case of an error during the execution of a query, mysql_query will return false -- which means you should test the value returned by that function -- to possibly log the cause of the error :

$result = mysql_query($query);
if ($result === false) {
    // An error has occured...
    echo mysql_error();
}

Note : here, I just displayed the error message -- but you should instead log the error somewhere (to a file, for instance), before putting your application to production : your users don't need (nor want) to see any technical error message !

Pascal MARTIN
Moreover, the insert is likely failing because some of those columns are ints (e.g. age) and he is passing them in as if they are strings, single-quoted. He's going to have to remove those single quotes around those and insure that only integer values occur in those variables.
Conspicuous Compiler
That might be true too *(in which case those data should be escaped with `intval` or `floatval`, for instance, to make sure they only contain numbers)* -- but I don't think that passing integers as strings would actually cause an SQL error.
Pascal MARTIN
@Conspicuous Compiler: depending on the mode the MySQL server is running in this may be an error but it may also be only a warning `Incorrect integer value: 'a' for column 'foo' at row 1`, see http://dev.mysql.com/doc/refman/5.1/en/server-sql-mode.html
VolkerK
It looks like a connection error. but is happening on my other form too, so im wondering if its my host.... and NO, i did not know i was vulnerable to the sql injecion attack. i just added that function. follow up, how do i keep a user from just typing the URL of this page. I have a link to a free gues pass PDF in the echo statement.how can i secure this page so that if no data is passed, it will display an error "sorry you didnt send us any information"..for instance.
tony noriega
A: 

Check the result from mysql_query(...) to see if it failed or not. If it didn't fail, MySQL should definitely have stored the information for you.

Ztyx