patternphpMinor
Possible security issues in email validation
Viewed 0 times
emailissuesvalidationpossiblesecurity
Problem
I need feedback on this code regarding security issues.
I'm using this
<?php
if(!isset($_POST['submit']))
{
//This page should not be accessed directly. Need to submit the form.
echo "error; you need to submit the form!";
}
$name = $_POST['name'];
$visitor_email = $_POST['email'];
$message = $_POST['message'];
//Validate first
if(empty($name)||empty($visitor_email))
{
echo "Name and email are mandatory!";
exit;
}
if(IsInjected($visitor_email))
{
echo "Bad email value!";
exit;
}
$email_from = 'someemail@some.com';//<== update the email address
$email_subject = "New Form submission";
$email_body = "You have received a new message from the user $name.\n".
"Here is the message:\n $message".
$to = "someemail@some.com";//<== update the email address
$headers = "From: $email_from \r\n";
$headers .= "Reply-To: $visitor_email \r\n";
//Send the email!
mail($to,$email_subject,$email_body,$headers);
//done. redirect to thank-you page.
header('Location: thank-you.html');I'm using this
IsInjected($str) function to validate email addresses:// Function to validate against any email injection attempts
function IsInjected($str)
{
$injections = array('(\n+)',
'(\r+)',
'(\t+)',
'(%0A+)',
'(%0D+)',
'(%08+)',
'(%09+)'
);
$inject = join('|', $injections);
$inject = "/$inject/i";
if(preg_match($inject,$str))
{
return true;
}
else
{
return false;
}
}
?>Solution
In your first validation, I think you forgot to exit, you probably meant:
To validate email addresses, you can use the
For example:
will output:
For more details and (caveats!), see the docs. Also some more examples here.
Your original
you could write simply:
if(!isset($_POST['submit']))
{
//This page should not be accessed directly. Need to submit the form.
echo "error; you need to submit the form!";
exit;
}To validate email addresses, you can use the
filter_var method, like this:function is_valid_email($str) {
return filter_var($str, FILTER_VALIDATE_EMAIL);
}For example:
function check_email($email) {
printf("checking '%s': %s\n", $email, is_valid_email($email) ? 'OK' : 'INVALID');
}
check_email('joe@example.com');
check_email('bogus');will output:
checking 'joe@example.com': OK
checking 'bogus': INVALIDFor more details and (caveats!), see the docs. Also some more examples here.
Your original
IsInjected method might still be useful for other purposes, that's why I created a new method for validating emails. If you keep it I would rename it to hasInjectedChars or has_injected_chars. And these lines can be improved:if(preg_match($inject,$str))
{
return true;
}
else
{
return false;
}you could write simply:
return preg_match($inject, $str);Code Snippets
if(!isset($_POST['submit']))
{
//This page should not be accessed directly. Need to submit the form.
echo "error; you need to submit the form!";
exit;
}function is_valid_email($str) {
return filter_var($str, FILTER_VALIDATE_EMAIL);
}function check_email($email) {
printf("checking '%s': %s\n", $email, is_valid_email($email) ? 'OK' : 'INVALID');
}
check_email('joe@example.com');
check_email('bogus');checking 'joe@example.com': OK
checking 'bogus': INVALIDif(preg_match($inject,$str))
{
return true;
}
else
{
return false;
}Context
StackExchange Code Review Q#49282, answer score: 3
Revisions (0)
No revisions yet.