Are there any open vulnerabilities in this mailer script?

I made a PHP mailer script does the basic validation of fields, return errors, else submit if all is good. But it also has a honeypot field that is not required to be filled in (I'm assuming by hiding it using CSS a spambot will fill in the field anyway). If the field is not empty, it opens a text file and writes/appends the attempt on it, and it also sends an email alert of the attempt.

```
H:i:s');

if(isset($_POST['_save'])) {
$name = stripslashes($_POST['name']);
$email = stripslashes($_POST['email']);
$company = stripslashes($_POST['company']);
$message = stripslashes($_POST['message']);
$subject = stripslashes($_POST['subject']);
$website = stripslashes($_POST['website']);

if (empty($name) || empty($email) || empty($subject) || empty($message) ||
!empty($website)) {
if (empty($name))
$error['name'] = "Please enter your Full Name";
if (empty($email))
$error['email'] = "Please enter a valid Email Address";
if (empty($company))
$error['company'] = "Please enter Your Company Name";
if (empty($subject))
$error['subject'] = "Please Write a Subject";
if (empty($message))
$error['message'] = "Please write a message, inquiries or
other concerns above";
if (!empty($website)) // this is the honeypot
$error['subject'] = "Opps looks like you're a spambot. You
just filled in a not required field.;
$myFile = "botlog.txt";
$fh = fopen($myFile, 'a') or die("can't open file");
$stringData = "bot trapped" . " " . "-" . " " . $website . " " . "-
" . " " . $current_date . "\r\n";
fwrite($fh, $stringData);
fclose($fh);
$donot="donotreply@example.com";
$headers="From: {$email}\r\nReply-To: {$donot}"; //create headers
mail('opps@example.com',$headers,$stringData);
}
else { //if not empty
stripslashes($headers);
$headers="From: {$email}\r\nReply-To: {$email}"; //c

Your mailer is vulnerable to a header-splitting attack! (Also known as a CRLF-injection attack.)

Basically, if one of the fields contains a CRLF, the attacker can insert any arbitrary header into the generated message. For example, if the email parameter is

victim1@example.com%0d%0aCc:%20victim2@example.net

… then you will compose a message with headers

From: victim1@example.com
Cc: victim2@example.net
Reply-To: victim1@example.com
Cc: victim2@example.net

PHP's mail() function will "helpfully" add victim2@example.net as a spam recipient. The spam will look like it came from victim1@example.com.

The simplest mitigation, I believe, is to pass all of the header fields through
mb_encode_mimeheader() first.

General warning regarding string concatenation and interpolation


Any time you compose a string that will be interpreted by another computer system, be it an e-mail message, HTML page, or SQL query, assume that you are vulnerable to some kind of injection attack. Every single one of these "human-friendly" languages (as opposed to binary formats such as JPEG images) will have delimiters of special significance. Header-splitting, HTML/JavaScript injection, and SQL injection attacks all have the same root cause: careless string concatenation or interpolation.


Therefore, before concatenating or interpolating strings, stop and think: "What is the appropriate escaping mechanism that I should be using?" Better yet, ask: "Is there a higher-level library that will let me accomplish the task without low-level string manipulation?"

RFC time!


I'm diving into a deeper technical discussion about why mb_encode_mimeheader() is my choice for escaping. If you get confused by this technical discussion, you can safely ignore this explanation and just filter all the input through mb_encode_mimeheader().


There are actually two relevant standards for e-mail headers. Mail Transfer Agents (MTAs) care about RFC 2822. Mail User Agents (MUAs) care about RFC 2047.


MTAs are completely oblivious to MIME encoding. As long as they see the headers they need (e.g. "From", "To", "Return-Path"), and the headers contain a safe subset of ASCII, they are happy.


MUAs, on the other hand, are more sophisticated, in that they have to handle non-ASCII text for the benefit of non-English users. For example, a "Subject" header could contain arbitrary text, such as "¡Hóla!". Such a Subject line should be encoded using

mb_encode_mimeheader("¡Hóla!", "UTF-8", "Q");

as

=?UTF-8?Q?=C2=A1H=C3=B3la!?=

That is, by the way, the way to ensure that the $subject parameter is RFC 2047-compliant, as specified in the mail() documentation.


What about headers that are relevant to both MTAs and MUAs, such as the "From" header? One might want to generate a header that looks to the user like

From: "François Müller" 

Over the wire, such a header must be encoded, since it contains non-ASCII characters. One acceptable encoding would be

From: "=?UTF-8?Q?Fran=C3=A7ois=20M=C3=BCller?=" 

Such an encoding satisfies both RFC 2822 (since the MTA sees an opaque ASCII string between the double-quotes) and RFC 2047 (since the MUA should know how to decode the MIME-encoded string for display). To achieve such a result, you would need PHP code such as

$from_header = sprintf('From: "%s" ',
                       mb_encode_mimeheader("François Müller", "UTF-8", "Q"),
                       "francois.mueller@example.com");

If you write your code as

$email = '"François Müller" ';
$from_header = mb_encode_mimeheader("From: $email", "UTF-8", "Q");

… you'll get

From: =?UTF-8?Q?=22Fran=C3=A7ois=20M=C3=BCller=22=20=3Cfrancois=2Emueller?=
 =?UTF-8?Q?=40example=2Ecom=3E?=

… which is wrong, since the MTA sees one opaque ASCII string that does not look like a valid e-mail address. However, the main point is that such code, though simplistic, is not vulnerable to header injection. As long as the $email variable contains simple ASCII text, it will work. For example,

$email = '';
$from_header = mb_encode_mimeheader("From: $email", "UTF-8", "Q");

… passes the text through unchanged, producing

From: 

PHP-bashing time!


As is often the case with PHP, the mail() function is poorly designed, such that it is easy to fall into this trap. Expecting programmers to know the RFCs and take the time to encode all of the headers properly is pure madness! If mail() were designed such that it accepted an associative array of (header name → header value) pairs, then it could automatically apply mitigation against header-splitting attacks.

StackExchange Code Review Q#43037, answer score: 19