|
| 1 | +From 181c44c7ff26b96a68afedb127eeb36adb745d50 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Kanishk-Bansal <kbkanishk975@gmail.com> |
| 3 | +Date: Thu, 28 Nov 2024 09:20:01 +0000 |
| 4 | +Subject: [PATCH] Fix CVE patch for CVE-2024-6923 in fasttrack/2.0 |
| 5 | + |
| 6 | +--- |
| 7 | + Doc/library/email.errors.rst | 6 +++ |
| 8 | + Doc/library/email.policy.rst | 18 ++++++++ |
| 9 | + Doc/whatsnew/3.9.rst | 12 ++++++ |
| 10 | + Lib/email/_header_value_parser.py | 12 ++++-- |
| 11 | + Lib/email/_policybase.py | 8 ++++ |
| 12 | + Lib/email/errors.py | 4 ++ |
| 13 | + Lib/email/generator.py | 13 +++++- |
| 14 | + Lib/test/test_email/test_generator.py | 62 +++++++++++++++++++++++++++ |
| 15 | + Lib/test/test_email/test_policy.py | 26 +++++++++++ |
| 16 | + 9 files changed, 157 insertions(+), 4 deletions(-) |
| 17 | + |
| 18 | +diff --git a/Doc/library/email.errors.rst b/Doc/library/email.errors.rst |
| 19 | +index f4b9f52..878c09b 100644 |
| 20 | +--- a/Doc/library/email.errors.rst |
| 21 | ++++ b/Doc/library/email.errors.rst |
| 22 | +@@ -59,6 +59,12 @@ The following exception classes are defined in the :mod:`email.errors` module: |
| 23 | + :class:`~email.mime.image.MIMEImage`). |
| 24 | + |
| 25 | + |
| 26 | ++.. exception:: HeaderWriteError() |
| 27 | ++ |
| 28 | ++ Raised when an error occurs when the :mod:`~email.generator` outputs |
| 29 | ++ headers. |
| 30 | ++ |
| 31 | ++ |
| 32 | + Here is the list of the defects that the :class:`~email.parser.FeedParser` |
| 33 | + can find while parsing messages. Note that the defects are added to the message |
| 34 | + where the problem was found, so for example, if a message nested inside a |
| 35 | +diff --git a/Doc/library/email.policy.rst b/Doc/library/email.policy.rst |
| 36 | +index bf53b95..57a75ce 100644 |
| 37 | +--- a/Doc/library/email.policy.rst |
| 38 | ++++ b/Doc/library/email.policy.rst |
| 39 | +@@ -229,6 +229,24 @@ added matters. To illustrate:: |
| 40 | + |
| 41 | + .. versionadded:: 3.6 |
| 42 | + |
| 43 | ++ |
| 44 | ++ .. attribute:: verify_generated_headers |
| 45 | ++ |
| 46 | ++ If ``True`` (the default), the generator will raise |
| 47 | ++ :exc:`~email.errors.HeaderWriteError` instead of writing a header |
| 48 | ++ that is improperly folded or delimited, such that it would |
| 49 | ++ be parsed as multiple headers or joined with adjacent data. |
| 50 | ++ Such headers can be generated by custom header classes or bugs |
| 51 | ++ in the ``email`` module. |
| 52 | ++ |
| 53 | ++ As it's a security feature, this defaults to ``True`` even in the |
| 54 | ++ :class:`~email.policy.Compat32` policy. |
| 55 | ++ For backwards compatible, but unsafe, behavior, it must be set to |
| 56 | ++ ``False`` explicitly. |
| 57 | ++ |
| 58 | ++ .. versionadded:: 3.9.20 |
| 59 | ++ |
| 60 | ++ |
| 61 | + The following :class:`Policy` method is intended to be called by code using |
| 62 | + the email library to create policy instances with custom settings: |
| 63 | + |
| 64 | +diff --git a/Doc/whatsnew/3.9.rst b/Doc/whatsnew/3.9.rst |
| 65 | +index 1756a37..eeda4e6 100644 |
| 66 | +--- a/Doc/whatsnew/3.9.rst |
| 67 | ++++ b/Doc/whatsnew/3.9.rst |
| 68 | +@@ -1625,3 +1625,15 @@ ipaddress |
| 69 | + |
| 70 | + * Fixed ``is_global`` and ``is_private`` behavior in ``IPv4Address``, |
| 71 | + ``IPv6Address``, ``IPv4Network`` and ``IPv6Network``. |
| 72 | ++ |
| 73 | ++email |
| 74 | ++----- |
| 75 | ++ |
| 76 | ++* Headers with embedded newlines are now quoted on output. |
| 77 | ++ |
| 78 | ++ The :mod:`~email.generator` will now refuse to serialize (write) headers |
| 79 | ++ that are improperly folded or delimited, such that they would be parsed as |
| 80 | ++ multiple headers or joined with adjacent data. |
| 81 | ++ If you need to turn this safety feature off, |
| 82 | ++ set :attr:`~email.policy.Policy.verify_generated_headers`. |
| 83 | ++ (Contributed by Bas Bloemsaat and Petr Viktorin in :gh:`121650`.) |
| 84 | +diff --git a/Lib/email/_header_value_parser.py b/Lib/email/_header_value_parser.py |
| 85 | +index 8a8fb8b..e394cfd 100644 |
| 86 | +--- a/Lib/email/_header_value_parser.py |
| 87 | ++++ b/Lib/email/_header_value_parser.py |
| 88 | +@@ -92,6 +92,8 @@ TOKEN_ENDS = TSPECIALS | WSP |
| 89 | + ASPECIALS = TSPECIALS | set("*'%") |
| 90 | + ATTRIBUTE_ENDS = ASPECIALS | WSP |
| 91 | + EXTENDED_ATTRIBUTE_ENDS = ATTRIBUTE_ENDS - set('%') |
| 92 | ++NLSET = {'\n', '\r'} |
| 93 | ++SPECIALSNL = SPECIALS | NLSET |
| 94 | + |
| 95 | + def quote_string(value): |
| 96 | + return '"'+str(value).replace('\\', '\\\\').replace('"', r'\"')+'"' |
| 97 | +@@ -2778,9 +2780,13 @@ def _refold_parse_tree(parse_tree, *, policy): |
| 98 | + wrap_as_ew_blocked -= 1 |
| 99 | + continue |
| 100 | + tstr = str(part) |
| 101 | +- if part.token_type == 'ptext' and set(tstr) & SPECIALS: |
| 102 | +- # Encode if tstr contains special characters. |
| 103 | +- want_encoding = True |
| 104 | ++ if not want_encoding: |
| 105 | ++ if part.token_type == 'ptext': |
| 106 | ++ # Encode if tstr contains special characters. |
| 107 | ++ want_encoding = not SPECIALSNL.isdisjoint(tstr) |
| 108 | ++ else: |
| 109 | ++ # Encode if tstr contains newlines. |
| 110 | ++ want_encoding = not NLSET.isdisjoint(tstr) |
| 111 | + try: |
| 112 | + tstr.encode(encoding) |
| 113 | + charset = encoding |
| 114 | +diff --git a/Lib/email/_policybase.py b/Lib/email/_policybase.py |
| 115 | +index c9cbadd..d1f4821 100644 |
| 116 | +--- a/Lib/email/_policybase.py |
| 117 | ++++ b/Lib/email/_policybase.py |
| 118 | +@@ -157,6 +157,13 @@ class Policy(_PolicyBase, metaclass=abc.ABCMeta): |
| 119 | + message_factory -- the class to use to create new message objects. |
| 120 | + If the value is None, the default is Message. |
| 121 | + |
| 122 | ++ verify_generated_headers |
| 123 | ++ -- if true, the generator verifies that each header |
| 124 | ++ they are properly folded, so that a parser won't |
| 125 | ++ treat it as multiple headers, start-of-body, or |
| 126 | ++ part of another header. |
| 127 | ++ This is a check against custom Header & fold() |
| 128 | ++ implementations. |
| 129 | + """ |
| 130 | + |
| 131 | + raise_on_defect = False |
| 132 | +@@ -165,6 +172,7 @@ class Policy(_PolicyBase, metaclass=abc.ABCMeta): |
| 133 | + max_line_length = 78 |
| 134 | + mangle_from_ = False |
| 135 | + message_factory = None |
| 136 | ++ verify_generated_headers = True |
| 137 | + |
| 138 | + def handle_defect(self, obj, defect): |
| 139 | + """Based on policy, either raise defect or call register_defect. |
| 140 | +diff --git a/Lib/email/errors.py b/Lib/email/errors.py |
| 141 | +index d28a680..1a0d5c6 100644 |
| 142 | +--- a/Lib/email/errors.py |
| 143 | ++++ b/Lib/email/errors.py |
| 144 | +@@ -29,6 +29,10 @@ class CharsetError(MessageError): |
| 145 | + """An illegal charset was given.""" |
| 146 | + |
| 147 | + |
| 148 | ++class HeaderWriteError(MessageError): |
| 149 | ++ """Error while writing headers.""" |
| 150 | ++ |
| 151 | ++ |
| 152 | + # These are parsing defects which the parser was able to work around. |
| 153 | + class MessageDefect(ValueError): |
| 154 | + """Base class for a message defect.""" |
| 155 | +diff --git a/Lib/email/generator.py b/Lib/email/generator.py |
| 156 | +index c9b1216..89224ae 100644 |
| 157 | +--- a/Lib/email/generator.py |
| 158 | ++++ b/Lib/email/generator.py |
| 159 | +@@ -14,12 +14,14 @@ import random |
| 160 | + from copy import deepcopy |
| 161 | + from io import StringIO, BytesIO |
| 162 | + from email.utils import _has_surrogates |
| 163 | ++from email.errors import HeaderWriteError |
| 164 | + |
| 165 | + UNDERSCORE = '_' |
| 166 | + NL = '\n' # XXX: no longer used by the code below. |
| 167 | + |
| 168 | + NLCRE = re.compile(r'\r\n|\r|\n') |
| 169 | + fcre = re.compile(r'^From ', re.MULTILINE) |
| 170 | ++NEWLINE_WITHOUT_FWSP = re.compile(r'\r\n[^ \t]|\r[^ \n\t]|\n[^ \t]') |
| 171 | + |
| 172 | + |
| 173 | + |
| 174 | +@@ -223,7 +225,16 @@ class Generator: |
| 175 | + |
| 176 | + def _write_headers(self, msg): |
| 177 | + for h, v in msg.raw_items(): |
| 178 | +- self.write(self.policy.fold(h, v)) |
| 179 | ++ folded = self.policy.fold(h, v) |
| 180 | ++ if self.policy.verify_generated_headers: |
| 181 | ++ linesep = self.policy.linesep |
| 182 | ++ if not folded.endswith(self.policy.linesep): |
| 183 | ++ raise HeaderWriteError( |
| 184 | ++ f'folded header does not end with {linesep!r}: {folded!r}') |
| 185 | ++ if NEWLINE_WITHOUT_FWSP.search(folded.removesuffix(linesep)): |
| 186 | ++ raise HeaderWriteError( |
| 187 | ++ f'folded header contains newline: {folded!r}') |
| 188 | ++ self.write(folded) |
| 189 | + # A blank line always separates headers from body |
| 190 | + self.write(self._NL) |
| 191 | + |
| 192 | +diff --git a/Lib/test/test_email/test_generator.py b/Lib/test/test_email/test_generator.py |
| 193 | +index 89e7ede..d29400f 100644 |
| 194 | +--- a/Lib/test/test_email/test_generator.py |
| 195 | ++++ b/Lib/test/test_email/test_generator.py |
| 196 | +@@ -6,6 +6,7 @@ from email.message import EmailMessage |
| 197 | + from email.generator import Generator, BytesGenerator |
| 198 | + from email.headerregistry import Address |
| 199 | + from email import policy |
| 200 | ++import email.errors |
| 201 | + from test.test_email import TestEmailBase, parameterize |
| 202 | + |
| 203 | + |
| 204 | +@@ -216,6 +217,44 @@ class TestGeneratorBase: |
| 205 | + g.flatten(msg) |
| 206 | + self.assertEqual(s.getvalue(), self.typ(expected)) |
| 207 | + |
| 208 | ++ def test_keep_encoded_newlines(self): |
| 209 | ++ msg = self.msgmaker(self.typ(textwrap.dedent("""\ |
| 210 | ++ To: nobody |
| 211 | ++ Subject: Bad subject=?UTF-8?Q?=0A?=Bcc: injection@example.com |
| 212 | ++ |
| 213 | ++ None |
| 214 | ++ """))) |
| 215 | ++ expected = textwrap.dedent("""\ |
| 216 | ++ To: nobody |
| 217 | ++ Subject: Bad subject=?UTF-8?Q?=0A?=Bcc: injection@example.com |
| 218 | ++ |
| 219 | ++ None |
| 220 | ++ """) |
| 221 | ++ s = self.ioclass() |
| 222 | ++ g = self.genclass(s, policy=self.policy.clone(max_line_length=80)) |
| 223 | ++ g.flatten(msg) |
| 224 | ++ self.assertEqual(s.getvalue(), self.typ(expected)) |
| 225 | ++ |
| 226 | ++ def test_keep_long_encoded_newlines(self): |
| 227 | ++ msg = self.msgmaker(self.typ(textwrap.dedent("""\ |
| 228 | ++ To: nobody |
| 229 | ++ Subject: Bad subject=?UTF-8?Q?=0A?=Bcc: injection@example.com |
| 230 | ++ |
| 231 | ++ None |
| 232 | ++ """))) |
| 233 | ++ expected = textwrap.dedent("""\ |
| 234 | ++ To: nobody |
| 235 | ++ Subject: Bad subject |
| 236 | ++ =?utf-8?q?=0A?=Bcc: |
| 237 | ++ injection@example.com |
| 238 | ++ |
| 239 | ++ None |
| 240 | ++ """) |
| 241 | ++ s = self.ioclass() |
| 242 | ++ g = self.genclass(s, policy=self.policy.clone(max_line_length=30)) |
| 243 | ++ g.flatten(msg) |
| 244 | ++ self.assertEqual(s.getvalue(), self.typ(expected)) |
| 245 | ++ |
| 246 | + |
| 247 | + class TestGenerator(TestGeneratorBase, TestEmailBase): |
| 248 | + |
| 249 | +@@ -224,6 +263,29 @@ class TestGenerator(TestGeneratorBase, TestEmailBase): |
| 250 | + ioclass = io.StringIO |
| 251 | + typ = str |
| 252 | + |
| 253 | ++ def test_verify_generated_headers(self): |
| 254 | ++ """gh-121650: by default the generator prevents header injection""" |
| 255 | ++ class LiteralHeader(str): |
| 256 | ++ name = 'Header' |
| 257 | ++ def fold(self, **kwargs): |
| 258 | ++ return self |
| 259 | ++ |
| 260 | ++ for text in ( |
| 261 | ++ 'Value\r\nBad Injection\r\n', |
| 262 | ++ 'NoNewLine' |
| 263 | ++ ): |
| 264 | ++ with self.subTest(text=text): |
| 265 | ++ message = message_from_string( |
| 266 | ++ "Header: Value\r\n\r\nBody", |
| 267 | ++ policy=self.policy, |
| 268 | ++ ) |
| 269 | ++ |
| 270 | ++ del message['Header'] |
| 271 | ++ message['Header'] = LiteralHeader(text) |
| 272 | ++ |
| 273 | ++ with self.assertRaises(email.errors.HeaderWriteError): |
| 274 | ++ message.as_string() |
| 275 | ++ |
| 276 | + |
| 277 | + class TestBytesGenerator(TestGeneratorBase, TestEmailBase): |
| 278 | + |
| 279 | +diff --git a/Lib/test/test_email/test_policy.py b/Lib/test/test_email/test_policy.py |
| 280 | +index e87c275..ff1ddf7 100644 |
| 281 | +--- a/Lib/test/test_email/test_policy.py |
| 282 | ++++ b/Lib/test/test_email/test_policy.py |
| 283 | +@@ -26,6 +26,7 @@ class PolicyAPITests(unittest.TestCase): |
| 284 | + 'raise_on_defect': False, |
| 285 | + 'mangle_from_': True, |
| 286 | + 'message_factory': None, |
| 287 | ++ 'verify_generated_headers': True, |
| 288 | + } |
| 289 | + # These default values are the ones set on email.policy.default. |
| 290 | + # If any of these defaults change, the docs must be updated. |
| 291 | +@@ -277,6 +278,31 @@ class PolicyAPITests(unittest.TestCase): |
| 292 | + with self.assertRaises(email.errors.HeaderParseError): |
| 293 | + policy.fold("Subject", subject) |
| 294 | + |
| 295 | ++ def test_verify_generated_headers(self): |
| 296 | ++ """Turning protection off allows header injection""" |
| 297 | ++ policy = email.policy.default.clone(verify_generated_headers=False) |
| 298 | ++ for text in ( |
| 299 | ++ 'Header: Value\r\nBad: Injection\r\n', |
| 300 | ++ 'Header: NoNewLine' |
| 301 | ++ ): |
| 302 | ++ with self.subTest(text=text): |
| 303 | ++ message = email.message_from_string( |
| 304 | ++ "Header: Value\r\n\r\nBody", |
| 305 | ++ policy=policy, |
| 306 | ++ ) |
| 307 | ++ class LiteralHeader(str): |
| 308 | ++ name = 'Header' |
| 309 | ++ def fold(self, **kwargs): |
| 310 | ++ return self |
| 311 | ++ |
| 312 | ++ del message['Header'] |
| 313 | ++ message['Header'] = LiteralHeader(text) |
| 314 | ++ |
| 315 | ++ self.assertEqual( |
| 316 | ++ message.as_string(), |
| 317 | ++ f"{text}\nBody", |
| 318 | ++ ) |
| 319 | ++ |
| 320 | + # XXX: Need subclassing tests. |
| 321 | + # For adding subclassed objects, make sure the usual rules apply (subclass |
| 322 | + # wins), but that the order still works (right overrides left). |
| 323 | +-- |
| 324 | +2.45.2 |
| 325 | + |
0 commit comments