patternphpMinor
Preventing email injection
Viewed 0 times
injectionpreventingemail
Problem
I have asked a question on Stack Overflow and one of the guys actually managed to hack my contact form and inject a fake email into the
So after talking to him, he sent me a few articles, and I aimed to update the PHP to secure this gap and prevent the spam.
Here is my old PHP:
and here is the updated version:
```
Invalid email address!";
break;
case !preg_match($spam_pattern, $name):
case !preg_match($spam_pattern, $number):
case !preg_match($spam_pattern, $date):
case !preg_match($spam_pattern, $select):
case !preg_match($spam_pattern, $radio):
case !preg_match($spam_pattern, $checkbox):
case !preg_match($spam_pattern, $message):
$response = "Invalid request made!";
break;
default:
$to = "";
$subject = "New Message From: $name";
$message = "Name: $name
number: $number
date: $date
select: $select
radio: $radio
$header of the PHP! So after talking to him, he sent me a few articles, and I aimed to update the PHP to secure this gap and prevent the spam.
Here is my old PHP:
number: $number
date: $date
select: $select
radio: $radio
checkbox: $checkbox
Email: $email
Message: $message";
$headers = 'MIME-Version: 1.0' . "\r\n";
$headers .= 'Content-type: text/html; charset=utf-8' . "\r\n";
$headers .= 'From: '.$email . "\r\n";
$mailed = (mail($to, $subject, $message, $headers));
if( isset($_REQUEST['ajax']))$response = ($mailed) ? "1" : "0";
else $response = ($mailed) ? "Success!" : "Error! There was a problem with sending.";
echo $response;
}
} else {
echo "Form data error!";
}
ob_flush();
die();
}
?>and here is the updated version:
```
Invalid email address!";
break;
case !preg_match($spam_pattern, $name):
case !preg_match($spam_pattern, $number):
case !preg_match($spam_pattern, $date):
case !preg_match($spam_pattern, $select):
case !preg_match($spam_pattern, $radio):
case !preg_match($spam_pattern, $checkbox):
case !preg_match($spam_pattern, $message):
$response = "Invalid request made!";
break;
default:
$to = "";
$subject = "New Message From: $name";
$message = "Name: $name
number: $number
date: $date
select: $select
radio: $radio
Solution
@James is correct, PHP has function to validate so you don't have to (and they're better).
Your
You don't need the parentheses in this line. In fact, you could just shorten it all a bit and use:
Your
switch (true) is bothering me. I think an if condition could work there instead, as that's not why switches were built.$mailed = (mail($to, $subject, $message, $headers));You don't need the parentheses in this line. In fact, you could just shorten it all a bit and use:
if (mail($to, $subject, $message, $headers)) {
$response = "Success!";
} else {
$response = "Error! There was a problem with sending.";
}Code Snippets
$mailed = (mail($to, $subject, $message, $headers));if (mail($to, $subject, $message, $headers)) {
$response = "<h2 style='color: green'>Success!</h2>";
} else {
$response = "<h2 style='color: blue'>Error! There was a problem with sending.</h2>";
}Context
StackExchange Code Review Q#42313, answer score: 5
Revisions (0)
No revisions yet.