Secure: Not yet, but it's a start. Your validation is too strict in some places, and may result in the wrong email address being used. In particular here:
$email = filter_var($_POST['email'], FILTER_SANITIZE_EMAIL);
//a bit later on:
$email = str_replace(array("\r", "\n", "%0a", "%0d"), '', stripslashes($email));
If the value of $email
is one of these examples of valid email addresses:
!#$%&'*+-/=?^_`{}|[email protected]
"()<>[]:,;@\\\"!#$%&'*+-/=?^_`{}| ~.a"@example.org
"very.(),:;<>[]\".VERY.\"very@\\ \"very\".unusual"@strange.example.com
you're stripping the slashes, which'll yield:
!#$%&'*+-=?^_`{}|[email protected]
"()<>[]:,;@"!#$%&'*+-=?^_`{}| ~.a"@example.org
"very.(),:;<>[]".VERY."very@ "very".unusual"@strange.example.com
Effectively changing the actual email address, and this may in turn turn a valid, though outlandish email address into an invalid one.
BTW: source for these email addresses is the email wiki
When it comes to email addresses: use filter_validate($emai, FILTER_VALIDATE_EMAIL)
and rely on that function's ability to validate the string. Don't do any processing of your own, especially not after having sanitized and validated the email address.
Bottom line, if you want to validate an email address, while allowing users to pass an (accidental) \r
char, use this:
$email = filter_var($_POST['email'], FILTER+_SANITIZE_EMAIL);
if (!filter_var($email, FILTER_VALIDATE_EMAIL))
{
echo $jsonHelper->encode($email.' is an invalid address'));
}
On the other hand, you're not sanitizing the message and name as well as maybe you should. It's not unlikely your script is vulnerable to mail injection. A must-read on this subject can be found here.
Since we're all in the business of programming, I hope you'll understand I'm not going to repeat myself here, but I've dealt with mail injection previously on this site: read my answer to this questionread my answer to this question, which focusses on injection a bit more.
Some other niggles:
- Please, as ever, try to stick to the coding standards as described by PHP-FIG. Most, if not all, major players subscribe to this standard, so should you. Check the standard here.
Things likepublic function foobar(){
are not accepted, the opening brace should go on the next line. It may seem silly to stumble over this, but really: it isn't. - don't use
die
in a method, and avoid usingecho
. I take it you're using a framework of sorts. Check if there's an ajax helper that outputs, or use the view to echo JSON content. And from within an action (controller method), don't calldie
, but ratherreturn
, signaling an uneventfull end of the controllers' job.die
breaks the normal way of processing a request, which may cause problems down the road.