4 ms·

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

by Synchro 10y ago

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.

Roritharr 10y ago
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?
- Piskvorrr 10y ago
  For benign e-mail addresses, it is Good Enough: accepts the unusual but valid info%blah+something@info.swiss, 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)
annnnd 10y ago
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)
- thaumaturgy 10y ago
  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 https://tools.ietf.org/html/rfc5321#section-4.1.2 (Email addresses suck and this is not me saying that this is sane.)
Piskvorrr 10y ago
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.

by Synchro 10y ago

Roritharr 10y ago

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?

Piskvorrr 10y ago
For benign e-mail addresses, it is Good Enough: accepts the unusual but valid info%blah+something@info.swiss, 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)

annnnd 10y ago

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)

thaumaturgy 10y ago
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 https://tools.ietf.org/html/rfc5321#section-4.1.2 (Email addresses suck and this is not me saying that this is sane.)

Piskvorrr 10y ago

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.