Inclusion of Functionality from Untrusted Control Sphere in phpmailer/phpmailer
Reported on
Jun 10th 2021
✍️ Description
validateAddress function used to validate email addresses, uses call_user_func to call the callable from the name of callable provided to the function as an argument $patternselect. But if no argument is passed, the function sets "php" as default value to $patternselect variable on line 1337 (ironically). If a callable (function) named php is present in the scope, the function gets executed.
🕵️♂️ Proof of Concept
<?php
use PHPMailer\PHPMailer\PHPMailer;
use PHPMailer\PHPMailer\SMTP;
use PHPMailer\PHPMailer\Exception;
require 'vendor/autoload.php';
function php() {
echo 'this was invoked by PHPMailer\'s validateAddress function';
}
$mail = new PHPMailer(true);
$mail->isSMTP();
$mail->Host = 'smtp.example.com';
$mail->setFrom('from@example.com', 'Mailer');
$mail->addAddress('to@example.com', 'Joe User');
$mail->Subject = 'Here is the subject';
$mail->Body = 'This is the message';
$mail->send();
💥 Impact
It can introduce a bug in the code if the developer is using a function named php and PHPMailer altogether. It would be hard to track this kinda bug. Also, if an attacker is able to inject a function by any means, he can utilize this to execute code stealthly.
Occurrences
Hey @0xCrypto! I've emailed the PHPMailer maintainer and waiting to hear back :)
Hi, thanks for the report.
I agree that this is an issue, but impact is very low since it requires that an attacker can already inject remote code in the app context, which is a much bigger problem. I don't agree with the proposed fix in the PR though. It would be better to filter the fixed names first via an allow-list; that way it can't be worked around by explicitly setting that value either. Like this on line 1340:
if (!in_array(strtolower($patternselect), ['pcre', 'pcre8', 'html5', 'php']) && is_callable($patternselect)) {
Also, in the name of coordinated disclosure, it's best not to create public PRs for unreleased issues. Early disclosure leads to mistakes made in haste.
Do you have a CVE number for this issue?
Hm, since the fix bounty is now "up for grabs", I wonder if I can apply for it since I just posted a fix!
@0xCrypto, I'm writing docs for the patch release and CVE – how would you like to be credited?
@Marcus, you can reward yourself the fix by clicking the confirm fix button, and you will see an option (Me).
Thanks @Jamie, Adam in chat told me that too. The fix I posted isn't quite right, but I have a better one already.
@Marcus, just a heads up that we have reserved a CVE ID for this advisory. The ID is:
CVE-2021-3603
Thanks Marcus. Yeah I always get confused by CVSS rating. For the CVE credition, Please credit it to my name "Vikrant Singh Chauhan" with email "vi@hackberry.xyz"
@0xcrypto, I've added you to the private repo for my proposed fix. Please could you take a look and confirm that it addresses this issue? It works the way I originally suggested, by denying names that match internal validators, but I also had to take into account injected functions that are not simple strings.
A patched version has been released as PHPMailer 6.5.0. Please update CVE-2021-34551 info. Release notes and other details are in the repo https://github.com/PHPMailer/PHPMailer/security/advisories/GHSA-77mr-wc79-m8j3 and https://github.com/PHPMailer/PHPMailer/blob/master/SECURITY.md
Thanks for the help and reports everyone!
For reference:
https://github.com/CVEProject/cvelist/pull/2037
We are awaiting a merge on this, and then the CVE should be published.