Skip to content

mail() could provide better protection against mail header injections #13402

Closed
@hannob

Description

@hannob

Description

The mail() function has an optional fourth parameter that allows setting additional mail headers. This can either be a newline-separated string or structured as an associative array.

It creates an obvious risk of header injection if one adds possibly untrusted content to the headers, and this is also documented.

In the string case this is more or less unavoidable. I.e. if you have something like this which could be used in a contact form:

mail($to, $subject, $msg, "Reply-To: ".$_POST['email']);

If $_POST['email'] is not validated before, you can add something like "[email protected]\r\nCc: [email protected]", injecting another recipient of that mail. This is, as said, unavoidable for this interface, and kinda expected.

However, in the array case, this still works, and this is more surprising, and could be avoided by PHP:

mail($to, $subject, $msg, ["Reply-To" => $_POST['email']]);

The same injection attack as above works here. (Update: slightly modified with '\n' instead of '\r\n', see comment below.) Given that each header line is its own entry in the array, PHP could avoid allowing a header injection by either not allowing newlines in a header entry or by filtering them out. According to the comments in the docs, it already does so for the to and subject parameter, it would be logical to do so for headers as well.

I think this is a simple change that would make the function more secure.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions