1

Made a small contact form on php, it gets $_POST variables and mails to me.

<form action="/myscript.php" method="post">

Small piece of code:

$subject = trim($_POST['subject']);
$comment = trim($_POST['comment']);
mail($email, $subject, $comment, $headers);

$email is mine mail address, $headers are usual.

There is no filtration for subject and comment. Can it be a potential security hole to my site?

My mail is placed on gmail.com. Can unfiltered mail from my site hurt me, when I open gmail interface in browser?

How should I filter all the variables? Maybe I wish echo some of them on my site, after sending an email. (like 'Thanks, %name% !')

James
  • 42,081
  • 53
  • 136
  • 161

3 Answers3

1

No, it's not that dangerous. Gmail doesn't trust the e-mails you receive, otherwise every spammer would be able to compromise you.

However, it's a good practice to, at least, check if the variables exist and if their length doesn't exceed the maximum.

EDIT It's possible that old versions of PHP were vulnerable to e-mail injection attacks, as described here. It would not compromise your site and your e-mail client should be able to handle malicious e-mails safely, but could potentially turn you into a spam relay.

New versions do not exhibit this vulnerability, because all the control characters (those below 0x20) are sanitized. You can do the same sanitation like this:

$subject = filter_input(INPUT_POST, "subject", FILTER_UNSAFE_RAW,
    FILTER_FLAG_STRIP_LOW);
if ($subject === false) { /* subject not given/not scalar; handle it */ }
Artefacto
  • 96,375
  • 17
  • 202
  • 225
  • what is the maximum length for mail fields? – James Aug 21 '10 at 19:36
  • 1
    Are you saying he should not sanityse his data because gmail does it for you? Don`t you think thats bad practice? – Iznogood Aug 21 '10 at 19:36
  • @Iznogood I'm saying that it both won't result in a vulnerability in his site and that, save for a bug in Gmail, it will also not result in a security risk when viewing the e-mail. See also my last paragraph. – Artefacto Aug 21 '10 at 19:39
  • 1
    @Work For the subject, see http://stackoverflow.com/questions/1592291/what-is-the-email-subject-length-limit For the body of the e-mail, there's no limit, but you probably don't want to be able to send yourself multi-megabyte e-mails. – Artefacto Aug 21 '10 at 19:40
  • @Artefacto out of curiosity if you where to read your gmail emails from another place (with pop3 for example) would gmail still clean up your data? Never tought about this before. – Iznogood Aug 21 '10 at 19:43
  • @Izn I don't think so. But anyway, all the e-mail clients should be able sanitize the e-mails. Actually, if he's sending the message as plain/text, there's very little opportunity to compromise anything. The reason I don't say "no opportunity at all" is because I don't know if `mail` strips the subject out of new line characters. If it doesn't, you should be able to inject your own headers and compose an arbitrary message. But I repeat, if this results in some compromise when reading the e-mail, it's a bug of the e-mail client. – Artefacto Aug 21 '10 at 19:58
  • @Izn I've checked the source code of `mail` and it checks for control characters in the subject. So it's not possible to inject new headers in the e-mail. – Artefacto Aug 21 '10 at 20:05
1

Yes, it is dangerous, vulnerable to attack called Mail injection.
Though it can not hurt your site but can be used by spammers. 

$subject = "Site feedback";
$comment = trim($_POST['subject'])."\n\n".trim($_POST['comment']);
mail($email, $subject, $comment);

this one would be safe.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Why would `trim` help you here? – Artefacto Aug 21 '10 at 19:42
  • Sorry, -1. This won't help you at all. – Artefacto Aug 21 '10 at 20:01
  • I explain. The information e.g. [here](http://www.securephpwiki.com/index.php/Email_Injection) is at least outdated. You cannot inject additional headers with `mail` because removes all the control characters from the subject (including new lines). See here http://lxr.php.net/opengrok/xref/PHP_TRUNK/ext/standard/mail.c#155 I removed the downvote though, because maybe old versions are vulnerable. – Artefacto Aug 21 '10 at 20:04
1

probably you could check http://swiftmailer.org/ a php mailer component-library in order to compare your solution with it. Swiftmailer is the mailer solution for frameworks such as symfony-project.org .

plain text is not an issue for a website, attachments are, but comment and subject would not create any problem in your server. regarding gmail, it has its own email verification consequently it would be difficult for an email with virus or similar to pass their analysis. rgds.

s_h
  • 1,476
  • 2
  • 27
  • 61