I am new to PHP and put this contact page together using the w3Schools.com example as a guide along with a couple other sources. As far as I can tell, it works fine, but before I put it up on my page I would like to know if there is any potential for abuse. Any feedback is appreciated.
<!DOCTYPE html>
<html>
<head>
<?php include 'header.php'; ?>
</head>
<body>
<?php
require_once "Mail.php";
function spamCheck($field) {
$field = filter_var($field, FILTER_SANITIZE_EMAIL);
if (filter_var($field, FILTER_VALIDATE_EMAIL)) {
return true;
} else {
return false;
}
}
if (isset($_REQUEST['email'])) {
$mailcheck = spamCheck($_REQUEST['email']);
if ($mailcheck == false) {
echo "Invalid Input";
} else {
$from = $_REQUEST['email'];
$body = "From\n" . $from . "\n\n" . $_REQUEST['message'];
$to = "<REMOVED>";
$subject = "$_REQUEST['subject'];
$host = "<REMOVED>";
$port = "<REMOVED>";
$username = "<REMOVED>";
$password = "<REMOVED>";
$headers = array('From' => $from, 'To' => $to, 'Subject' => $subject);
$smtp = Mail::factory('smtp',array ('host' => $host,
'port' => $port,
'auth' => true,
'username' => $username,
'password' => $password));
$mail = $smtp->send($to, $headers, $body);
if (PEAR::isError($mail)) {
echo("<p>" . $mail->getMessage() . "</p>");
} else {
echo("<p>Message successfully sent!</p>");
}
}
} else {
echo "<h1>Send me an email if you like.</h1>
<br><form method='post' action='contact.php'><br>
<p>Your Email: <input name='email' type='text'>
Subject: <input name='subject' type='text'></p>
<p>Message:<br>
<textarea name='message' rows='15' cols='40'>
</textarea><br>
<input type='submit'></p>
</form>";
}
?>
</body>
<?php include "footer.php"?>
</html>
3 Answers 3
You're spamCheck function isn't really doing a proper check. It's removing all the invalid characters from the e-mail address and then checking if it's valid, so even if the e-mail had invalid characters it would still passes.
For example:
$dangerousEmail = "localhost\nCc:[email protected]";
// This will output "Email is valid!" even though it isn't
if(spamCheck($dangerousEmail))
echo "Email is valid!";
A better check would be just to use filter_var with FILTER_VALIDATE_EMAIL directly:
if (filter_var($email, FILTER_VALIDATE_EMAIL))
{
// Sanitize the e-mail to be extra safe.
// I think Pear Mail will automatically do this for you
$email = filter_var($field, FILTER_SANITIZE_EMAIL);
echo "Email is valid!";
}
-
\$\begingroup\$ This is exactly the find of feedback I was hoping for. Thank you very much. I see the error in my logic. \$\endgroup\$Guy– Guy2013年05月22日 04:05:30 +00:00Commented May 22, 2013 at 4:05
-
\$\begingroup\$ Is it safe to assume that I should validate THEN sanitize in all cases? \$\endgroup\$Guy– Guy2013年05月22日 04:16:24 +00:00Commented May 22, 2013 at 4:16
I use a blank input field in the form and hide it with css. If the field exists upon form submit, I know it's spam and just ignore it.
<form method="post">
<input type="text" name="email" />
<input type="text" name="email2" style="display:none;" />
<input type="submit" />
</form>
<?php if (!$_POST['email2']) { // not spam } ?>
Of course this only works with bots and spammers who scrape sites to get all the input fields, but since I've implemented it, it's worked great.
In case anyone else is interested I was able to increase the security somewhat. After some research, I think that I have made it more secure, from spammers, at least. I implemented a captcha by Securimage. It was pretty simple to use and I tested it out and think it does a fine job. They have good documentation and a simple quickstart guide right here. I am going to spend some more time learning about it, but it was quick and easy to get up and running.