[php-src] Issue #13402: mail() could provide better protection against mail header injections
| From: | hannob | Date: | Thu, 15 Feb 2024 18:16:42 +0000 |
| Subject: | [php-src] Issue #13402: mail() could provide better protection against mail header injections | ||
| Groups: | php.bugs | ||
| Request: | Send a blank email to php-bugs+get-246463@lists.php.net to get a copy of this message | ||
Issue: https://github.com/php/php-src/issues/13402
Author: 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
"foo@example.com\r\nCc: hacker@example.com", 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. 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.