Hacker News new | past | comments | ask | show | jobs | submit login
PHPMailer Exploit – Remote Code Execution (legalhackers.com)
181 points by dawid_golunski on Dec 28, 2016 | hide | past | favorite | 96 comments



The root cause of this is that PHP's mail function is broken by design. Instead of parameterized values for everything, it passes the entirety of the "additional" options, which includes the from address, as one string for the shell to parse. If the flags were pulled out to individual options to be passed to the command instead, it wouldn't be possible to exploit things in the way it does. So, instead of:

    mail ( string $to , string $subject , string $message [, string $additional_headers [, string $additional_parameters ]] )

it would be something like:

    mail ( string $to , string $subject , string $message [string $additional_headers], [string $additional_parameter, string $parameter values ] )
With the result passed to sendmail via the underlying functions, not relying on the shell to separate options for you. Any sort of user-supplied data should be parameterized and treated differently than data you provide. We've mostly learned our lessons from SQL injection, the rest of the stack still has a problem.


As far as I can tell, PHP doesn't have any way to spawn a subprocess without passing it to /bin/sh for evaluation.

PHPMailer (not core php) apparently either calls php's popen(), which passes to the shell...or calls php's mail(), which uses popen(). There are other options in php, like proc_open(), but they also call /bin/sh.

TLDR: There isn't any way in PHP to avoid "relying on the shell to separate options for you".


It's possible with pcntl_fork() and pcntl_exec() but that's not compatible with apache's mod_php which is probably still pretty widely used even though it's getting replaced by php-fpm.


I've read reports on the incompatibilities, but the odd part is that you can popen which, under FreeBSD at least, is a fairly thin wrapper around vfork() and execve() makes that feel suspect. It may be a bit more work, but commands like this still should be treated as special.


I believe many distributions disable pcntl_* functions even in a php-fpm environment. You can make it work, of course, but it's not the default.


Per the writeup, it looks like they were already escaping their arguments, but they didn't know PHP's built-in mail() function was already doing that:

   PHPMailer 5.2.17 sanitizes the $Sender variable
   by applying escapeshellarg() escaping before the
   value is passed to mail() function.

   It does not however take into account the clashing
   of the escapeshellarg() function with internal
   escaping with escapeshellcmd() performed by mail()
   function on the 5th parameter.

   As a result it is possible to inject an extra quote
   that does not get properly escaped and break out of
   the escapeshellarg() protection applied by the patch
   in PHPMailer 5.2.17.
In most cases, people are using PHP as a server-side language under a web server. In those cases, it really isn't a good idea to send mail in-process anyway due to web-process and SMTP timeouts. Queueing messages with something like Gearman or some other job handler would be a more secure and performant approach.


> It looks like better documentation could have prevented this bug.

Where?

Barring user-submitted comments, PHP has consistently had some of the most complete programming language documentation.

This exact issue is specifically documented with the mail() function[1] so I'm not sure you can blame PHP or ask for better documentation in this case:

"This parameter is escaped by escapeshellcmd() internally to prevent command execution. escapeshellcmd() prevents command execution, but allows to add additional parameters. For security reasons, it is recommended for the user to sanitize this parameter to avoid adding unwanted parameters to the shell command."

Escaping too many times or incorrectly is a pretty common error but it's normally of the PEBKAC variety.

[1] - http://php.net/manual/en/function.mail.php


Why is it possible to do command execution in a mailer at all??? Any reasonable language would have abstracted this into an SMTP library where such things are impossible instead of relying on sendmail.


Likely because PHP has to run things while the end user is waiting for a page to download. They rely on sendmail so that the mail can be queued, versus hanging while the email is sent. PHPmailer can be configured in the way you're describing, but it then has this issue.


Opening a socket to a local MTA and dumping a message to it wouldn't take any longer than shelling out to a local sendmail and writing to its stdin.


That's true, but it doesn't typically work out of the box. Postfix on debian and ubuntu, for example, won't allow relay through localhost....unless it's sasl authenticated, which requires work.

So, yes, you can, but software like Wordpress includes a default configuration that works for most people without additional work...piping to /usr/bin/sendmail.


You're right. I retract the documentation comment.


> In those cases, it really isn't a good idea to send mail in-process anyway due to web-process and SMTP timeouts

Why is sending a message to something like RabbitMQ less likely to timeout then to postfix?


Job queue services have async modes where enqueueing the message returns immediately. Then the jobber can send the message under a more limited account, or even on a different machine in a different language without timeouts.

Last I checked, PHP's mail() function blocks until SMTP connect/auth/submit completes, and with things like SMTP tarpitting, or just ordinary slowness, that can take a very long time.

Sometimes, it takes more time than your server-side web process is alloted for execution, leading to your script being force-terminated prior to completion.

[in reply to skarap] The action of mail() depends on the mail/sendmail settings in your php.ini, so the behavior could be blocking or non-blocking depending on which client is used and which parameters are passed.


Both postfix and qmail are inherently incapable of blocking their clients on communication with destination/upstream smarthost because of their architecture.

In both cases it works like this:

    client: MAIL FROM
    server checks whether it makes sense (outgoing smarthost has essentially nothing to check here)
    SMTP server: 250 Ok
    client: RCPT TO
    server checks, again there is nothing expensive to check for smarthost case
    SMTP server: 250 Ok
    client: DATA ... .
    server does its checks on the contents of message (again, nothing meaningful for smarthos), reformats it, adds Received and such and writes it out into queue
    SMTP server: 250 Ok: queued
    server wakes up the process that handles delivery by IPC and gives it queue ID of just created message
    client probably disconnects now
    outgoing SMTP daemon starts connecting to somewhere


exim and sendmail have synchronous options


I'm aware of that. My point is that when sending mail directly from web server process is too slow, correct solution is not building your own bug-ridden implementation of half of SMTP on top of some newfangled message queue, but simply configuring your outgoing mail server correctly.

When you only enqueue event such as "this happened, there should probably be an notification for that" and have some non-trivial application logic in the queue runner it starts to make sense. But queue runner of the kind for(;;) {msg = get_message(); smtp_send(message)} is complete nonsense.

By the way, I know of pretty significant line of bussiness system that has nothing to do with email except the fact that it uses smtp and postfix as it's message bus and the thing seems to just work without issue, for more than decade.


Never said to re-implement SMTP. Just stating that I've been on shared servers where PHP mail() blocks and had to job-out SMTP for responsiveness. If I've encountered it, I'm sure others have as well.

As for the jobber, it was using the same PHP mail() function, it's just that the jobber runs async and does not have a time limit imposed on it.

If you have control over your php.ini to where you can set agent parameters to be non-blocking, then yes, that would be ideal.


The default configuration for PHP (and I guess the most frequently used one) is using sendmail command. That command does just one thing - enqueue the message on the local server. It doesn't handle the delivery of the email.

That is also the case when using the mail server at 127.0.0.1 port 25.

One could have configured a remote SMTP server with authentication and that would be slow and would be affected by network issues, but same would happen with a remote RabbitMQ server.


Quite the contrary: most installs I've seen have a "remote" dedicated SMTP server which is actually on local network (not significantly slower than mail()), and does other useful stuff like DKIM and SPF (which, with appropriate setup, has the added bonus that rogue app-server emails don't just appear out of nowhere, and at the very minimum have to pass through this server, giving you a place to debug). Of course, if you're making My First Blog Server on a shared host, then all of this is moot.


On most shared hosts, it blocks. The action taken by mail() is dependent upon your php.ini settings.


Claiming it is a responsible disclosure, because of a post at a forum somewhere does not make it responsible.

Edit: I should probably clarify that it is the 1 day timeline over Christmas that I think is irresponsible.


It's not been patched yet.

Edit: Seems like another exploit found 8 hours ago: https://github.com/PHPMailer/PHPMailer/issues/924

Probably wise to disable phpmailer on your servers for now.


[flagged]


That doesn't help - user input is already validated by default in PHPMailer - the problem is that a valid email address can also be an attack string in a shell context, and bugs in PHP make it hard to work around safely.


That's actually a part of the problem: "don't worry, the library covers you, no need to think about it". I have spent some time rooting around inside the libraries I planned to use - including PHPMailer - to find out what their assumptions are: "validates by default" is not a sufficient description (as we see), "passes everything conformant to RFC" is.

In this case, that was not the same assumption I was making, and I was surprised - once, during implementation; a wrapper using filter_var brought the assumptions of the library in line with the assumptions for the project. That is, obviously, not universal advice.


I wonder why they don't provide a mitigation. It shouldn't be impossible to release a simple sanitizeAgainstCVE_2016_10045($email) function as a mitigation that people could right now plop into their applications until phpmailer gets fixed... or is filter_var($email,FILTER_VALIDATE_EMAIL) good enough?


For benign e-mail addresses, it is Good Enough: accepts the unusual but valid info%[email protected], punycode (and various other things that people usually toss out with the infamous [a-z]{2,4}), yet throws out the abovementioned monstrosity, even though it technically follows the letter of the RFC.

As for "why no mitigation" - even though it abstracts the horror of mail(), a library is a power tool with sharp edges, not a nicely wrapped single-button selfie app. In other words, it already does validation; input sanitization is beyond its scope IMNSHO (this literally means that valid, insane e-mail addresses do exist), and adding that by default would nerf the library. (Also, php_filter is an extension and not installed by default)


Really? So you're saying this is a valid e-mail address?

    "\"Attacker\\' -Param2 -Param3\"@test.com"
Let's try it:

    <?php
      $sender = "\"Attacker\\' -Param2 -Param3\"@test.com";
      var_dump(filter_var($sender, FILTER_VALIDATE_EMAIL));
    // outputs:
    // bool(false)


filter_var(...) explicitly does not strictly adhere to the RFCs.

And yes, that is, technically, a valid email address:

    "\"Attacker\\' -Param2 -Param3\"@test.com"
gets interpreted by the shell (or by PHP, or by whatever else is responsible for handling backslash-escaped entities in quoted strings) as

    "Attacker\' -Param2 -Param3"@test.com
and "Attacker\' -Param2 -Param3" is a valid local-part: https://tools.ietf.org/html/rfc5321#section-4.1.2

(Email addresses suck and this is not me saying that this is sane.)


Out of curiosity, why is PHPMailer invoking the command line at all?

It would seem much safer to establish a TCP connection to the local MTA over port 25 or 587 and send the message that way.

Admins can just use iptables to restrict access to the port to localhost, and/or do the same in their MTA config.


It's using PHP's builtin mail() function, which happens to be a wrapper around /usr/bin/sendmail, and one of the parameters it takes is used as an "additional command-line parameter to sendmail". In particular, the "-f" switch is used to set the SMTP envelope MAIL FROM address. Apparently PHP's mail() tries to do some automagic shell escaping on the argument but it's likely not good enough and it also creates a conflict when callers such as PHPMailer also try to escape the parameter.


Software should never be using -f with user emails, regardless if it's sanitized or not. -f sets the return path, which will trigger DMARC protection if the domain doesn't allow it. For example, the yahoo.com policy will reject all mail if you try to set the return path to the user's yahoo.com address using -f.

The best way I've found to send mail on behalf of someone else is to leave the return path (-f) and From header pointing to your own domain, and use the Reply-To header with the users email.


PHPMailer can be configured to send mail through raw SMTP, by directly invoking sendmail, or by calling PHP's mail() function (which is a thin wrapper around sendmail). This vuln affects the last mode, where the command line is invoked by PHP itself. Yes, a good admin would configure mail differently, but this being PHP, it tries to be flexible and support the simplest options possible.


Using command line is the standard way in Linux and de-facto standard in *nix world.

"To deliver electronic mail (email), applications shall support the interface provided by sendmail (described here). This interface shall be the default delivery method for applications."

http://refspecs.linux-foundation.org/LSB_3.0.0/LSB-PDA/LSB-P...


Yes but PHP runs on non-*NIX platforms[0]. So why not use an interface which is (a) more secure, and (b) platform independent?

[0] http://windows.php.net/


Why do anything that makes sense? All I can say about the PHP maintainers' decision process without swearing is that it bewilders me.

Last time I tried configuring a Linux-hosted PHP to do SMTP on localhost:25 instead of shelling out to call sendmail, I found that only Windows builds even have that functionality compiled in. You can apply the same configuration options on a stock Linux build, and they won't cause errors, but they won't do anything, either. Maybe that's changed recently, but it would have to have been very recently, because I ran into this (for the umpteenth time) just a few months ago, while reworking my team's dev environment to allow for examining sent mail without requiring heroism.

I'm not a PHP hater, exactly. I don't like the language at all, but I understand it well and have made a very good living based partly on that knowledge. But the mail story in PHP has never not been a dumpster fire.


There are hundreds of ready made libraries which help you easily talk to SMTP without invoking a shell cmd. You also have the option to use sock functions (fsockopen) and write your connection wrapper. These are not windows-only features.


Of course. The part I still can't get my head around is where the language feature that'd relieve me of the need to vet a library, or worse write socket code by hand like some kind of barbarian, isn't compiled in for non-Windows platforms, because reasons.


¯\_(ツ)_/¯ PHP. This is one of the insane decisions made in PHP4 that stays there because it's always been there.




Stock wordpress shouldn't be remotely exploitable with this. The exploit relies on the end user being able to specify the "From" address. That is passed to sendmail via the -f parameter on the command line, which is why the vulnerability exists. Other addresses, like To:, are passed via the headers/piped, so they don't create issues.

Wordpress is adding a fix, but I assume that's to cover plugins that allow end users to set the From: address, like perhaps "Share this with a friend" type functionality where the email is meant to look like it's from a different domain.

In short, I don't think most wordpress installations are remotely exploitable via this bug.


Looks like recent versions of WordPress may or may not reject emails with the quoted name format of "bad stuff"@example.com. Might depend on your plugins. My experimentation produced varied results for my sites and testbeds.

filter_var($email, FILTER_SANITIZE_EMAIL) works for this exploit, as it removes spaces and double quotes.

The SMTP plugins I surveyed still use PHPMailer.

You'd want to try something like:

  /**
   * Block the PHPMailer vulnerability: 
   * https://legalhackers.com/advisories/PHPMailer-Exploit-Remote-Code-Exec-CVE-2016-10045-Vuln-Patch-Bypass.html
   */
  function example_wp_mail_filter($args) {
    $new_wp_mail = array(
      # Get rid of quotes in quoted emails: "bad stuff"@example.com. Should be
      # sufficient sabotage.
      'to' => preg_replace('[\'"]/u', "", $args['to']),
      'subject' => $args['subject'],
      'message' => $args['message'],
      'headers' => $args['headers'],
      'attachments' => $args['attachments'],
    );
  
    return $new_wp_mail;
  }
  add_filter('wp_mail', 'example_wp_mail_filter');


Oh geez. That does block the exploit, but breaks the functionality. `"Random A. Person" <[email protected]>`

(Also, if the SMTP plugin uses PhpMailer, but actually is configured to talk to SMTP, there is no mail() and the issue is moot)


It's times like this I'm glad I have a country block setup through ipdeny. Reducing the likelihood of attack by two orders of magnitude is a big help until there's a patch.


No it is unfortunately not if there exists at least one company that provides a VPN service from your country. :)


You don't think there's any value in reducing penetration attempts by 2-3 orders of magnitude? I went from seeing an attack every few seconds to one per day/week.


How do you monitor attacks?


Nice.

What countries do you block..?


The advisory says exploitation is not limited to just systems running the original Sendmail MTA, but Postfix' "sendmail" wrapper apparently ignores the "-X" parameter... so how can a Postfix-based system be exploited?


Yeah, looks like you're right, and although there might be some ways to abuse other Postfix command line options for some kind of mail abuse, I don't see that it offers any options that'll write a file to a specific location. If true, that's another big point in Postfix's favor.

On one of my web-facing servers running Postfix:

    root@:/home/rob# sendmail -X/home/rob/test.log
    sendmail: fatal: unsupported: -X/


I believe it's because the exploit isn't actually exploiting sendmail, it's expoiting /bin/sh sh -c <cmd>.

Php's implementation of popen() doesn't invoke commands directly with stdlib's execl() or execle()...it calls stdlib's popen(), which passes it through /bin/sh -c <cmd>.

That's assuming phpmailer doesn't treat a postfix sendmail wrapper differently than sendmail.

Edit: As far as I can tell, php doesn't allow any way to spawn a process without passing it to /bin/sh. That's odd, as other script languages like Perl and Python provide that. It avoids a whole class of exploits.

Edit2: It seems to be exploiting the "-f" option, not "-X". The postfix wrapper supports that.


It's exploiting -f to abuse -X to allow it to save a file with PHP tags to an externally-accessible location. Without -X or some similar capability, it's not exploitable. Well, it is, but not for the same amount of fun -- all you get to do is abuse whatever other arguments Postfix makes available to you.

I don't see a whole lot that a non-root user calling Postfix's version of sendmail can do with this ... maybe get a mail server RBL'd.


Ah, yes, you're right. Maybe the option to use an alternate alias database could be troublesome, but would require some other hole that allowed local file creation.


This discussion is broken by design. Using user input from contact form as "From" address for emails, sent from your site, is the mistake of your application - you should use something like "[email protected]", why you put user email there? In addition - most probably such emails will not pass spam filters.


It depends on whether you want to follow RFCs or not. Particularly, RFC 4021 [1] states that:

  Header Field: From

      Specifies the author(s) of the message; that is, the mailbox(es)
      of the person(s) or system(s) responsible for the writing of the
      message.  Defined as standard by RFC 822.

  Header Field: Sender

      Specifies the mailbox of the agent responsible for the actual
      transmission of the message.  Defined as standard by RFC 822.
As far as I understand, this means that yes, you should set the From address to the user's email when sending content submitted by that user. In contrast, you should never set the Sender address to any user's email.

Another discussion is whether this is sensible or not, how well it works delivery-wise, etc... In any case, it is not as clear-cut as you make it seem.

[1] https://tools.ietf.org/html/rfc4021#section-2.1.2


Right, whenever using mail() even without PHPMailer or anything for a basic form I've always thought it was best to set it to something like the following

  $headers = "From: [email protected]\n";
If you want to be able to reply to the email you can still set it with something like

  $headers .= "Reply-To: $email_address";
Then in your content body you could also include your $email_address variable.


You would probably want to respond to the user, and it is possible only if they give you their email in the form. Although, this can be solved by treating the From field in the mail form as just a field of information and have that in the body of email. But that would make automated replies difficult.


you could use reply-to header instead of from, being another additional header i guess it has the same problem.


Isn't this really a vulnerability of php's mail() function (or at least evidence of bad design). It shouldn't let itself call a shell command with arguments unescaped.


It appears that it does escape shell commands. The issue is that PHPMailer had already escaped them. The exploit is exploiting the "double escape".

The real issue to me is that unlike Perl and Python, PHP doesn't provide a way to spawn a process without invoking /bin/sh. If PHP had support for the various incantations of exec(), you could pass arguments without needing the shell.


I wonder if WordPress SMTP plugins¹ that override normal mail functions provide any protection from this vulnerability.

[1]: Such as https://wordpress.org/plugins/wp-mail-smtp/


Yes. This is only exploitable if you're using `mail()` to invoke the local MTA; if you're talking to a SMTP host, this exploit has nothing to work with.


My brain instinctively did s/PHP/CVE/, hope it's right.


....


TL;DR: PHPmailer fails to properly sanitize input. When configured to use CLI sendmail, this can lead to arbitrary command execution.

But then again, why would you send e-mail to just any address someone enters, without validating it for correctness? I know it is tough to validate all addresses according to RFC, but I'd rather block some legitimate users (which btw. probably know more about RFC than me and I'm sure can find a "nicer" e-mail address if they want to) than let some attacker use some vulnerability like this. Always whitelist valid input, never (just) blacklist it.

Also, I am baffled that frameworks I encountered never demanded from developer to specify exactly what kind of input it expects via POST & co.. It is trivial to write a set of functions like this:

    function input_post_email($field_name, $default_value)
    function input_post_string($field_name, $validation_regex, $default_value)
    ...
The point here is that framework should DEMAND from developer to specify format of each and every input var it needs. It should be difficult to bypass these restrictions, to demotivate developers doing it.

This is a first thing I made in every PHP project I started. Combined with Content Security Policy and output filtering it's... well, better than most other solutions. :)


Misguided developers incorrectly "validating" email addresses is the reason that one of my clients couldn't use one of the newer TLDs, ".place", and ended up scrapping it and trying to find something they wanted a lot less in the much more crowded .com space.

It's also why I far-too-often run into forms that won't let me use a "+" in the username part of my email address, which I use to track who's responsible for sending my email account off to third parties (e.g., "rob+paypal@....").

Some kinds of email validation are better than others. Using regular expressions and strictly adhering to the RFC is the one that developers are usually talking about when they say not to do it. filter_var(..., FILTER_VALIDATE_EMAIL) is sort of okay, although there are lots of edge cases that it doesn't handle correctly.


Genuinly curious: does '+' trick work? I would imagine any infringing parties would learn to remove plus sign and all after it before selling the database of addresses by now...

Other than that: true, I hate incorrect validation. But I hate sloppy security practices even more.


Not as much as it used to. You're right, they've caught on to it. I need to stand up another mail server pretty soon, I want to try making it easy to generate random recipient aliases, so e.g. [email protected] is an alias for [email protected] and [email protected] is an alias for [email protected]. That oughtta stump 'em for a while.


Sometimes not allowing a "+" is an anti-abuse feature. If you have a service tied to a customer providing a unique email address then you don't want them able to use a "+".


That seems like a strange place to verify that a user is only allowed to create one account. After all, they could just create a second email account on a free email provider and use that to sign up.

To get around that, the service provider would have to verify identity further down the line, making the email format restriction redundant.


I have been down this route and you also have to disable the use of free email accounts.

If the service value is not too high then even making people who want to abuse the service go through the process of registering a free email account works. Put a little bit of friction into the process and the script kiddies move onto an easier target.


> I have been down this route and you also have to disable the use of free email accounts.

I don't have any other email. Seems like a really bad idea.


It depends on the service. Some services are only aimed at business which have their own domain, but yes I agree this is a pretty drastic step to take.


I somehow went through life not knowing the + trick. Thanks, and commenting to hopefully highlight this for anyone else who didn't know.


The specific character depends on your MTA configuration. In Postfix this is the recipient_delimiter setting. I went with a "+" because that's the same character Gmail uses. Other mail services may not support this feature. Hope this helps!


This is incorrect. Input sanitisation is not the problem here - PHPMailer sanitises and validates email addresses well enough. The problem is that a valid, sanitised email address can also be an attack in an inappropriate shell context. Much as an SQL injection attack string may be harmless in a shell context but lethal in SQL. Also the vuln does not apply to CLI sendmail, only when sending via the PHP mail() function, and it's at least partly due to bugs in PHP itself.


I am not blaming lack of input sanitization, the bug is clearly still there. What I am saying is that input sanitization would prevent this attack because it will only allow valid characters to get to your mailer, which will exclude spaces, tabs, double and single quotes. Thus I fail to see an exploit vector which would bypass proper input sanitization. Do you see it?

In general I agree that PHP mail() function should take a part of blame here for not exposing a sane interface. Spammers have abused many bugs where programmers failed to sanitize "To:" or "From:" addresses and attackers could pass \r and \n characters (which allowed adding BCC fields, effectively sending e-mails to arbitrary addresses).

> Also the vuln does not apply to CLI sendmail...

Sure it does: >> ...will result in the followig list of arguments passed to sendmail program:

EDIT: you probably meant to say that the vulnerability is only triggered when CLI sendmail is called via mail(), which is true. Configuring PHPMail to use sendmail via CLI directly apparently doesn't expose this vulnerability.


> What I am saying is that input sanitization would prevent this attack because it will only allow valid characters to get to your mailer,

you have to be careful with the meaning of "valid" though: What's valid in one context might not be in another context. Do you want to limit the character set to the least common denominator of non-special characters valid in all possible contexts your address might be used?

What if you don't exclude some character because it's not special in any of the current contexts but then you later add another thing to the mix that uses in-band signalling? Now you need to update your whitelist and remove even more "invalid" characters.

This won't end well for you.

I would recommend you don't put a restriction on the input (aside of what the RFC defines as valid) and instead correctly escape (or throw if your context doesn't support escaping) when moving the raw data to a new context.

In this case, I think the problem is in PHP's `mail()` function that put stuff in shell context without any escaping.

The fix should be happing in `mail()` (which is internally shelling out and thus switching context), not in the caller and certainly not in the frontend controller when it needs to decide whether a given email address is valid or not.


I think you are taking my argument too far. Valid input doesn't mean it is safe to use in any context. It just means that anything which is invalid at the start shouldn't even have the chance to get in the system.

You should still properly escape data when passed to another context, no doubt about it.

I am also not suggesting "dirty" practices like replacing double quotes (Wordpress) or magically escaping quotes on input (PHP prior to... 5?). But if you know the input should be a number, allow only digits and '.' and clean everything else. And validate the range too! If you need a database ID, validate form and existence. If you need an e-mail address, run it through filter_var. Always. There is no reason not to.


> When configured to use CLI sendmail,

That's not correct. PHPMailer can be configured to send mail through raw SMTP, by directly invoking sendmail, or by calling PHP's mail() function (which is itself a wrapper around sendmail). This vulnerability affects only the last mode, when PHP's mail() calls sendmail. If you have PHPMailer configured to call sendmail directly, this vulnerability does not apply.


I stand corrected, thank you for clarifying.


>I know it is tough to validate all addresses according to RFC

Understatement of the year!

If you were to try and actually validate according to the RFC, chances are you'll introduce new vectors of attack in your code simply because of the complexity of what you're trying to achieve.

Honestly, the email address RFC is a litany of insane choices and kludged-together standards to account for the wild west free-for-all that existed before the modern internet emerged, and most of it has no bearing on the reality of how people use and create email addresses today. Better to just check if the address supplied has an @ in it somewhere and be done with it.


That's how I do it. Check for an @. Anything past that is sendmail's problem.


Even though it false positives some valid emails, I've always felt requiring a @ and a . is a good check for public facing validation.


Technically valid e-mail addresses - but nonsensical ones. I haven't seen an e-mail without a domain cross a network in decades ("cross a network" excepts "user@localhost", and I'm not old enough to have seen any IP-adressed mails ;)).

So yes, I believe that in 2016, a match on ^[^@]+@([^.@]+\.)+[^.@]+\.?$ does not have any actual false negatives, although allowing false positives. That's not entirely helpful in this CVE, though.


This is true, I couldn't figure out why this wasn't working, then realized I had to comment out this validation:

  filter_var($app->request()->post('email'), FILTER_VALIDATE_EMAIL)


[flagged]


Please try to avoid making generalisations about entire languages. I don't believe that comments like this have any place (or serve any purpose) in this community. Needless to say, PHP is still widely preferred by a great number of developers (who adhere to common standards) for building APIs and websites.


[flagged]


I am curious why you linked to reddit? There is nothing in the discussion (4 comments) that is not in the linked advisory.

Looking back through your comment history you seem to do this a lot, even linking to discussion pages on reddit with no comments whatsoever and the same link as found in the HN submission.


Hi

Could you please refrain from doing that?

> Ok, but to me it was ok as it was a link to more content: resolution, discussions with the author opened by the author of the discovery: dawid_golunski.

https://www.reddit.com/r/netsec/comments/5kot1a/phpmailer_52...


The author does not participate in any of the discussion there. Furthermore when you posted the link it only had 4 or 5 comments that added nothing to the discussion.

The best example is this one:

Https://news.ycombinator.com/item?id=13196875

It's the same exact link with no comments. What's the point?


[flagged]


I'd suspect that some of the websites it links to generate ad-rev

Did you find any? The 'simple logic' has to follow from some sort of evidence.


We're jerks and just strip everything down to a-Z space .- and @ _ anything beyond that f u, extensions and 3rd party library or even core filters come out with these vulnerabilities all the time, at least we will know for sure what characters were passed in from the start though I'm sure most business NEED to support every wacky combination but I'll take the complaint over the hack any day.


So you are one of those sites that reject email addresses with "+" in it?


That's a less than ideal solution. As another comment mentioned, you are screwing over legit email addresses with things like +. Better is to use better tooling. For sending email, I would use SendGrid or an alternative.


[email protected] is not amused. Might as well say "we throw out every e-mail that contains the letter 'b', because Reasons. Makes us feel warm, fuzzy and secure."




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: