SecurityPatch 9652: Possible problems after SUPEE-9652 applied
It's a super tiny patch, here's the diff:
diff --git lib/Zend/Mail/Transport/Sendmail.php lib/Zend/Mail/Transport/Sendmail.php
index b24026b..9323f58 100644
--- lib/Zend/Mail/Transport/Sendmail.php
+++ lib/Zend/Mail/Transport/Sendmail.php
@@ -119,14 +119,19 @@ class Zend_Mail_Transport_Sendmail extends Zend_Mail_Transport_Abstract
);
}
- set_error_handler(array($this, '_handleMailErrors'));
- $result = mail(
- $this->recipients,
- $this->_mail->getSubject(),
- $this->body,
- $this->header,
- $this->parameters);
- restore_error_handler();
+ // Sanitize the From header
+ if (!Zend_Validate::is(str_replace(' ', '', $this->parameters), 'EmailAddress')) {
+ throw new Zend_Mail_Transport_Exception('Potential code injection in From header');
+ } else {
+ set_error_handler(array($this, '_handleMailErrors'));
+ $result = mail(
+ $this->recipients,
+ $this->_mail->getSubject(),
+ $this->body,
+ $this->header,
+ $this->parameters);
+ restore_error_handler();
+ }
}
if ($this->_errstr !== null || !$result) {
However, Peter O'Callaghan (the one and only) seems to have found a bug. He gently shared the details with me and said I could share it with you here so here it is:
Best I can tell the value of
$this->params
will always be prefixed-f
at the point the validation has been added (it’s passed into the constructor at the point the return path is added). Therefore at the point it’s passed to the validation, if I’ve configured my e-mail[email protected]
, the value that’s actually being validated is[email protected]
, it seems more of a fluke than an intention that this happens to validate as an e-mail address. If my e-mail address was"example"@example.com
though, this would become-f"example"@example.com
, which won’t validate. Incidentally thestr_replace
seems completely redundant in this matter given that AFAIK a space can only be used in conjunction with quotes, and e-mails with quotes won’t validate with the-f
prefix. In fact if it wasn’t for the prefix being there, the str_replace and validate wouldn’t be useful because"foo bar"@example.com
and"foobar"@example.com
both validate, since the latter is never assigned to anything after the replacement, the e-mail would still be sent using the former value, which would presumably still be vulnerable.
Two other things to keep in mind:
- The CE version of the patch also has 'EE_1.14.3.1' listed as the version. It doesn't affect applying, just adds wrong version to
app/etc/applied.patches.list
it feels a bit odd. (source: https://twitter.com/JohnHughes1984/status/829050203139358720) - As long as you have the email settings disabled (see here: https://magento.com/security/news/new-zend-framework-1-security-vulnerability), there's no hurry with this one BUT you'll have to do it because any new patch will require this to be installed (as they go on top of each other).
Side note
The corresponding new release of Magento CE 1.9.3.2 also includes the copyright comment year update (from 2016 to 2017) so almost every files of Magento has been updated and the diff looks huge
Little tip for upgrading; after copying the new version over your existing install, run git diff -w --stat=400 | grep -v " 2 +”
to quickly see diffs that contain more changes than just the copyright notice change.
The Security Patch 9652 affects only the following file:
/lib/Zend/Mail/Transport/Sendmail.php