1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
|
From: Markus Koschany <apo@debian.org>
Date: Sat, 21 Jan 2017 14:54:57 +0100
Subject: CVE-2016-10074
The mail transport (aka Swift_Transport_MailTransport) in Swift Mailer allowed
remote attackers to pass extra parameters to the mail command
and consequently execute arbitrary code via a \" (backslash double quote) in a
crafted e-mail address in the From, ReturnPath, or Sender header.
Bug-Debian: https://bugs.debian.org/849626
Bug-Upstream: https://github.com/swiftmailer/swiftmailer/issues/844
Origin: https://github.com/swiftmailer/swiftmailer/commit/e6ccf40d856af9598b76eb313b215eed25ae9e86
---
lib/classes/Swift/Transport/MailTransport.php | 52 +++++++++++++++++++++++-
tests/unit/Swift/Transport/MailTransportTest.php | 27 ++++++++++++
2 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/lib/classes/Swift/Transport/MailTransport.php b/lib/classes/Swift/Transport/MailTransport.php
index ef3bc09..722da58 100644
--- a/lib/classes/Swift/Transport/MailTransport.php
+++ b/lib/classes/Swift/Transport/MailTransport.php
@@ -166,7 +166,7 @@ class Swift_Transport_MailTransport implements Swift_Transport
}
if ($this->_invoker->mail($to, $subject, $body, $headers,
- sprintf($this->_extraParams, $reversePath))) {
+ $this->_formatExtraParams($this->_extraParams, $reversePath))) {
if ($evt) {
$evt->setResult(Swift_Events_SendEvent::RESULT_SUCCESS);
$evt->setFailedRecipients($failedRecipients);
@@ -223,4 +223,54 @@ class Swift_Transport_MailTransport implements Swift_Transport
return $path;
}
+
+ /**
+ * Fix CVE-2016-10074 by disallowing potentially unsafe shell characters.
+ *
+ * Note that escapeshellarg and escapeshellcmd are inadequate for our purposes, especially on Windows.
+ *
+ * @param string $string The string to be validated
+ *
+ * @return bool
+ */
+ private function _isShellSafe($string)
+ {
+ // Future-proof
+ if (escapeshellcmd($string) !== $string || !in_array(escapeshellarg($string), array("'$string'", "\"$string\""))) {
+ return false;
+ }
+
+ $length = strlen($string);
+ for ($i = 0; $i < $length; ++$i) {
+ $c = $string[$i];
+ // All other characters have a special meaning in at least one common shell, including = and +.
+ // Full stop (.) has a special meaning in cmd.exe, but its impact should be negligible here.
+ // Note that this does permit non-Latin alphanumeric characters based on the current locale.
+ if (!ctype_alnum($c) && strpos('@_-.', $c) === false) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Return php mail extra params to use for invoker->mail.
+ *
+ * @param $extraParams
+ * @param $reversePath
+ *
+ * @return string|null
+ */
+ private function _formatExtraParams($extraParams, $reversePath)
+ {
+ if (false !== strpos($extraParams, '-f%s')) {
+ if (empty($reversePath) || false === $this->_isShellSafe($reversePath)) {
+ $extraParams = str_replace('-f%s', '', $extraParams);
+ } else {
+ $extraParams = sprintf($extraParams, $reversePath);
+ }
+ }
+ return !empty($extraParams) ? $extraParams : null;
+ }
+
}
diff --git a/tests/unit/Swift/Transport/MailTransportTest.php b/tests/unit/Swift/Transport/MailTransportTest.php
index 89cb2d5..b36076b 100644
--- a/tests/unit/Swift/Transport/MailTransportTest.php
+++ b/tests/unit/Swift/Transport/MailTransportTest.php
@@ -84,6 +84,33 @@ class Swift_Transport_MailTransportTest extends \SwiftMailerTestCase
$transport->send($message);
}
+ public function testTransportSettingInvalidFromEmail()
+ {
+ $invoker = $this->_createInvoker();
+ $dispatcher = $this->_createEventDispatcher();
+ $transport = $this->_createTransport($invoker, $dispatcher);
+
+ $headers = $this->_createHeaders();
+ $message = $this->_createMessageWithRecipient($headers);
+
+ $message->shouldReceive('getReturnPath')
+ ->zeroOrMoreTimes()
+ ->andReturn(
+ '"attacker\" -oQ/tmp/ -X/var/www/cache/phpcode.php "@email.com'
+ );
+ $message->shouldReceive('getSender')
+ ->zeroOrMoreTimes()
+ ->andReturn(null);
+ $message->shouldReceive('getFrom')
+ ->zeroOrMoreTimes()
+ ->andReturn(null);
+ $invoker->shouldReceive('mail')
+ ->once()
+ ->with(\Mockery::any(), \Mockery::any(), \Mockery::any(), \Mockery::any(), null);
+
+ $transport->send($message);
+ }
+
public function testTransportUsesHeadersFromMessage()
{
$invoker = $this->_createInvoker();
|
