File 0001-Prevent-CRLF-injections-described-in-CVE-2... of Package python-Twisted

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-Prevent-CRLF-injections-described-in-CVE-2019-12387.patch of Package python-Twisted (Revision 3c3db51246bcf79f688f0f5b4df615b5)

Currently displaying revision 3c3db51246bcf79f688f0f5b4df615b5 , Show latest

From 47af055a8a5e8a801a4d6dcc0493713ff0fb36a4 Mon Sep 17 00:00:00 2001
From: Mark Williams <mrw@enotuniq.org>
Date: Wed, 5 Jun 2019 00:03:37 -0700
Subject: [PATCH] Prevent CRLF injections described in CVE-2019-12387

Author: markrwilliams

Reviewers: glyph

Fixes: ticket:9647

Twisted's HTTP client APIs were vulnerable to maliciously constructed
HTTP methods, hosts, and/or paths, URI components such as paths and
query parameters.  These vulnerabilities were beyond the header name
and value injection vulnerabilities addressed in:

https://twistedmatrix.com/trac/ticket/9420
https://github.com/twisted/twisted/pull/999/

The following client APIs will raise a ValueError if given a method,
host, or URI that includes newlines or other disallowed characters:

- twisted.web.client.Agent.request
- twisted.web.client.ProxyAgent.request
- twisted.web.client.Request.__init__
- twisted.web.client.Request.writeTo

ProxyAgent is patched separately from Agent because unlike other
agents (e.g. CookieAgent) it is not implemented as an Agent wrapper.

Request.__init__ checks its method and URI so that errors occur closer
to their originating input.  Request.method and Request.uri are both
public APIs, however, so Request.writeTo (via Request._writeHeaders)
also checks the validity of both before writing anything to the wire.

Additionally, the following deprecated client APIs have also been
patched:

- twisted.web.client.HTTPPageGetter.__init__
- twisted.web.client.HTTPPageDownloader.__init__
- twisted.web.client.HTTPClientFactory.__init__
- twisted.web.client.HTTPClientFactory.setURL
- twisted.web.client.HTTPDownloader.__init__
- twisted.web.client.HTTPDownloader.setURL
- twisted.web.client.getPage
- twisted.web.client.downloadPage

These have been patched prior to their removal so that they won't be
vulnerable in the last Twisted release that includes them.  They
represent a best effort, because testing every combination of these
public APIs would require more code than deprecated APIs warrant.

In all cases URI components, including hostnames, are restricted to
the characters allowed in path components.  This mirrors the CPython
patch (for bpo-30458) that addresses equivalent vulnerabilities:

https://github.com/python/cpython/commit/bb8071a4cae5ab3fe321481dd3d73662ffb26052

HTTP methods, however, are checked against the set of characters
described in RFC-7230.

(cherry picked from commit 6c61fc4503ae39ab8ecee52d10f10ee2c371d7e2)
---
 twisted/web/_newclient.py             |  85 ++++-
 twisted/web/client.py                 |  25 +-
 twisted/web/newsfragments/9647.bugfix |   1 +
 twisted/web/test/injectionhelpers.py  | 168 ++++++++++
 twisted/web/test/test_agent.py        | 250 +++++++++++++-
 twisted/web/test/test_webclient.py    | 454 +++++++++++++++++++++++++-
 6 files changed, 970 insertions(+), 13 deletions(-)
 create mode 100644 twisted/web/newsfragments/9647.bugfix
 create mode 100644 twisted/web/test/injectionhelpers.py

diff --git a/twisted/web/_newclient.py b/twisted/web/_newclient.py
index c3f080d5a..c79bdf43a 100644
--- a/twisted/web/_newclient.py
+++ b/twisted/web/_newclient.py
@@ -29,6 +29,8 @@ Various other classes in this module support this usage:
 from __future__ import division, absolute_import
 __metaclass__ = type
 
+import re
+
 from zope.interface import implementer
 
 from twisted.python import log
@@ -551,6 +553,74 @@ class HTTPClientParser(HTTPParser):
 
 
 
+_VALID_METHOD = re.compile(
+    br"\A[%s]+\Z" % (
+        bytes().join(
+            (
+                b"!", b"#", b"$", b"%", b"&", b"'", b"*",
+                b"+", b"-", b".", b"^", b"_", b"`", b"|", b"~",
+                b"\x30-\x39",
+                b"\x41-\x5a",
+                b"\x61-\x7A",
+            ),
+        ),
+    ),
+)
+
+
+
+def _ensureValidMethod(method):
+    """
+    An HTTP method is an HTTP token, which consists of any visible
+    ASCII character that is not a delimiter (i.e. one of
+    C{"(),/:;<=>?@[\\]{}}.)
+
+    @param method: the method to check
+    @type method: L{bytes}
+
+    @return: the method if it is valid
+    @rtype: L{bytes}
+
+    @raise ValueError: if the method is not valid
+
+    @see: U{https://tools.ietf.org/html/rfc7230#section-3.1.1},
+        U{https://tools.ietf.org/html/rfc7230#section-3.2.6},
+        U{https://tools.ietf.org/html/rfc5234#appendix-B.1}
+    """
+    if _VALID_METHOD.match(method):
+        return method
+    raise ValueError("Invalid method {!r}".format(method))
+
+
+
+_VALID_URI = re.compile(br'\A[\x21-\x7e]+\Z')
+
+
+
+def _ensureValidURI(uri):
+    """
+    A valid URI cannot contain control characters (i.e., characters
+    between 0-32, inclusive and 127) or non-ASCII characters (i.e.,
+    characters with values between 128-255, inclusive).
+
+    @param uri: the URI to check
+    @type uri: L{bytes}
+
+    @return: the URI if it is valid
+    @rtype: L{bytes}
+
+    @raise ValueError: if the URI is not valid
+
+    @see: U{https://tools.ietf.org/html/rfc3986#section-3.3},
+        U{https://tools.ietf.org/html/rfc3986#appendix-A},
+        U{https://tools.ietf.org/html/rfc5234#appendix-B.1}
+    """
+    if _VALID_URI.match(uri):
+        return uri
+    raise ValueError("Invalid URI {!r}".format(uri))
+
+
+
 @implementer(IClientRequest)
 class Request:
     """
@@ -589,8 +659,8 @@ class Request:
             connection, defaults to C{False}.
         @type persistent: L{bool}
         """
-        self.method = method
-        self.uri = uri
+        self.method = _ensureValidMethod(method)
+        self.uri = _ensureValidURI(uri)
         self.headers = headers
         self.bodyProducer = bodyProducer
         self.persistent = persistent
@@ -635,8 +705,15 @@ class Request:
         # method would probably be good.  It would be nice if this method
         # weren't limited to issueing HTTP/1.1 requests.
         requestLines = []
-        requestLines.append(b' '.join([self.method, self.uri,
-            b'HTTP/1.1\r\n']))
+        requestLines.append(
+            b' '.join(
+                [
+                    _ensureValidMethod(self.method),
+                    _ensureValidURI(self.uri),
+                    b'HTTP/1.1\r\n',
+                ]
+            ),
+        )
         if not self.persistent:
             requestLines.append(b'Connection: close\r\n')
         if TEorCL is not None:
diff --git a/twisted/web/client.py b/twisted/web/client.py
index 9aff6683f..466716164 100644
--- a/twisted/web/client.py
+++ b/twisted/web/client.py
@@ -46,6 +46,9 @@ from twisted.web import error
 from twisted.web.iweb import UNKNOWN_LENGTH, IAgent, IBodyProducer, IResponse
 from twisted.web.http_headers import Headers
 
+from twisted.web._newclient import _ensureValidURI, _ensureValidMethod
+
+
 
 class PartialDownloadError(error.Error):
     """
@@ -77,11 +80,13 @@ class HTTPPageGetter(http.HTTPClient):
 
     _completelyDone = True
 
-    _specialHeaders = set((b'host', b'user-agent', b'cookie', b'content-length'))
+    _specialHeaders = set(
+        (b'host', b'user-agent', b'cookie', b'content-length'),
+    )
 
     def connectionMade(self):
-        method = getattr(self.factory, 'method', b'GET')
-        self.sendCommand(method, self.factory.path)
+        method = _ensureValidMethod(getattr(self.factory, 'method', b'GET'))
+        self.sendCommand(method, _ensureValidURI(self.factory.path))
         if self.factory.scheme == b'http' and self.factory.port != 80:
             host = self.factory.host + b':' + intToBytes(self.factory.port)
         elif self.factory.scheme == b'https' and self.factory.port != 443:
@@ -350,7 +355,7 @@ class HTTPClientFactory(protocol.ClientFactory):
             # just in case a broken http/1.1 decides to keep connection alive
             self.headers.setdefault(b"connection", b"close")
         self.postdata = postdata
-        self.method = method
+        self.method = _ensureValidMethod(method)
 
         self.setURL(url)
 
@@ -377,6 +382,7 @@ class HTTPClientFactory(protocol.ClientFactory):
         return "<%s: %s>" % (self.__class__.__name__, self.url)
 
     def setURL(self, url):
+        _ensureValidURI(url.strip())
         self.url = url
         uri = URI.fromBytes(url)
         if uri.scheme and uri.host:
@@ -700,7 +706,7 @@ def _makeGetterFactory(url, factoryFactory, contextFactory=None,
 
     @return: The factory created by C{factoryFactory}
     """
-    uri = URI.fromBytes(url)
+    uri = URI.fromBytes(_ensureValidURI(url.strip()))
     factory = factoryFactory(url, *args, **kwargs)
     if uri.scheme == b'https':
         from twisted.internet import ssl
@@ -1337,6 +1343,12 @@ class _AgentBase(object):
         Issue a new request, given the endpoint and the path sent as part of
         the request.
         """
+        if not isinstance(method, bytes):
+            raise TypeError('method={!r} is {}, but must be bytes'.format(
+                    method, type(method)))
+
+        method = _ensureValidMethod(method)
+
         # Create minimal headers, if necessary:
         if headers is None:
             headers = Headers()
@@ -1554,6 +1566,7 @@ class Agent(_AgentBase):
 
         @see: L{twisted.web.iweb.IAgent.request}
         """
+        uri = _ensureValidURI(uri.strip())
         parsedURI = URI.fromBytes(uri)
         try:
             endpoint = self._getEndpoint(parsedURI)
@@ -1587,6 +1600,8 @@ class ProxyAgent(_AgentBase):
         """
         Issue a new request via the configured proxy.
         """
+        uri = _ensureValidURI(uri.strip())
+
         # Cache *all* connections under the same key, since we are only
         # connecting to a single destination, the proxy:
         key = ("http-proxy", self._proxyEndpoint)
diff --git a/twisted/web/newsfragments/9647.bugfix b/twisted/web/newsfragments/9647.bugfix
new file mode 100644
index 000000000..b76916cdc
--- /dev/null
+++ b/twisted/web/newsfragments/9647.bugfix
@@ -0,0 +1 @@
+All HTTP clients in twisted.web.client now raise a ValueError when called with a method and/or URL that contain invalid characters.  This mitigates CVE-2019-12387.  Thanks to Alex Brasetvik for reporting this vulnerability.
\ No newline at end of file
diff --git a/twisted/web/test/injectionhelpers.py b/twisted/web/test/injectionhelpers.py
new file mode 100644
index 000000000..ffeb8629c
--- /dev/null
+++ b/twisted/web/test/injectionhelpers.py
@@ -0,0 +1,168 @@
+"""
+Helpers for URI and method injection tests.
+
+@see: U{CVE-2019-12387}
+"""
+
+import string
+
+
+UNPRINTABLE_ASCII = (
+    frozenset(range(0, 128)) -
+    frozenset(bytearray(string.printable, 'ascii'))
+)
+
+NONASCII = frozenset(range(128, 256))
+
+
+
+class MethodInjectionTestsMixin(object):
+    """
+    A mixin that runs HTTP method injection tests.  Define
+    L{MethodInjectionTestsMixin.attemptRequestWithMaliciousMethod} in
+    a L{twisted.trial.unittest.SynchronousTestCase} subclass to test
+    how HTTP client code behaves when presented with malicious HTTP
+    methods.
+
+    @see: U{CVE-2019-12387}
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt to send a request with the given method.  This should
+        synchronously raise a L{ValueError} if either is invalid.
+
+        @param method: the method (e.g. C{GET\x00})
+
+        @param uri: the URI
+
+        @type method:
+        """
+        raise NotImplementedError()
+
+
+    def test_methodWithCLRFRejected(self):
+        """
+        Issuing a request with a method that contains a carriage
+        return and line feed fails with a L{ValueError}.
+        """
+        with self.assertRaises(ValueError) as cm:
+            method = b"GET\r\nX-Injected-Header: value"
+            self.attemptRequestWithMaliciousMethod(method)
+        self.assertRegex(str(cm.exception), "^Invalid method")
+
+
+    def test_methodWithUnprintableASCIIRejected(self):
+        """
+        Issuing a request with a method that contains unprintable
+        ASCII characters fails with a L{ValueError}.
+        """
+        for c in UNPRINTABLE_ASCII:
+            method = b"GET%s" % (bytearray([c]),)
+            with self.assertRaises(ValueError) as cm:
+                self.attemptRequestWithMaliciousMethod(method)
+            self.assertRegex(str(cm.exception), "^Invalid method")
+
+
+    def test_methodWithNonASCIIRejected(self):
+        """
+        Issuing a request with a method that contains non-ASCII
+        characters fails with a L{ValueError}.
+        """
+        for c in NONASCII:
+            method = b"GET%s" % (bytearray([c]),)
+            with self.assertRaises(ValueError) as cm:
+                self.attemptRequestWithMaliciousMethod(method)
+            self.assertRegex(str(cm.exception), "^Invalid method")
+
+
+
+class URIInjectionTestsMixin(object):
+    """
+    A mixin that runs HTTP URI injection tests.  Define
+    L{MethodInjectionTestsMixin.attemptRequestWithMaliciousURI} in a
+    L{twisted.trial.unittest.SynchronousTestCase} subclass to test how
+    HTTP client code behaves when presented with malicious HTTP
+    URIs.
+    """
+
+    def attemptRequestWithMaliciousURI(self, method):
+        """
+        Attempt to send a request with the given URI.  This should
+        synchronously raise a L{ValueError} if either is invalid.
+
+        @param uri: the URI.
+
+        @type method:
+        """
+        raise NotImplementedError()
+
+
+    def test_hostWithCRLFRejected(self):
+        """
+        Issuing a request with a URI whose host contains a carriage
+        return and line feed fails with a L{ValueError}.
+        """
+        with self.assertRaises(ValueError) as cm:
+            uri = b"http://twisted\r\n.invalid/path"
+            self.attemptRequestWithMaliciousURI(uri)
+        self.assertRegex(str(cm.exception), "^Invalid URI")
+
+
+    def test_hostWithWithUnprintableASCIIRejected(self):
+        """
+        Issuing a request with a URI whose host contains unprintable
+        ASCII characters fails with a L{ValueError}.
+        """
+        for c in UNPRINTABLE_ASCII:
+            uri = b"http://twisted%s.invalid/OK" % (bytearray([c]),)
+            with self.assertRaises(ValueError) as cm:
+                self.attemptRequestWithMaliciousURI(uri)
+            self.assertRegex(str(cm.exception), "^Invalid URI")
+
+
+    def test_hostWithNonASCIIRejected(self):
+        """
+        Issuing a request with a URI whose host contains non-ASCII
+        characters fails with a L{ValueError}.
+        """
+        for c in NONASCII:
+            uri = b"http://twisted%s.invalid/OK" % (bytearray([c]),)
+            with self.assertRaises(ValueError) as cm:
+                self.attemptRequestWithMaliciousURI(uri)
+            self.assertRegex(str(cm.exception), "^Invalid URI")
+
+
+    def test_pathWithCRLFRejected(self):
+        """
+        Issuing a request with a URI whose path contains a carriage
+        return and line feed fails with a L{ValueError}.
+        """
+        with self.assertRaises(ValueError) as cm:
+            uri = b"http://twisted.invalid/\r\npath"
+            self.attemptRequestWithMaliciousURI(uri)
+        self.assertRegex(str(cm.exception), "^Invalid URI")
+
+
+    def test_pathWithWithUnprintableASCIIRejected(self):
+        """
+        Issuing a request with a URI whose path contains unprintable
+        ASCII characters fails with a L{ValueError}.
+        """
+        for c in UNPRINTABLE_ASCII:
+            uri = b"http://twisted.invalid/OK%s" % (bytearray([c]),)
+            with self.assertRaises(ValueError) as cm:
+                self.attemptRequestWithMaliciousURI(uri)
+            self.assertRegex(str(cm.exception), "^Invalid URI")
+
+
+    def test_pathWithNonASCIIRejected(self):
+        """
+        Issuing a request with a URI whose path contains non-ASCII
+        characters fails with a L{ValueError}.
+        """
+        for c in NONASCII:
+            uri = b"http://twisted.invalid/OK%s" % (bytearray([c]),)
+            with self.assertRaises(ValueError) as cm:
+                self.attemptRequestWithMaliciousURI(uri)
+            self.assertRegex(str(cm.exception), "^Invalid URI")
diff --git a/twisted/web/test/test_agent.py b/twisted/web/test/test_agent.py
index e694653d2..330f58517 100644
--- a/twisted/web/test/test_agent.py
+++ b/twisted/web/test/test_agent.py
@@ -11,7 +11,7 @@ from StringIO import StringIO
 
 from zope.interface.verify import verifyObject
 
-from twisted.trial.unittest import TestCase
+from twisted.trial.unittest import TestCase, SynchronousTestCase
 from twisted.web import client, error, http_headers
 from twisted.web._newclient import RequestNotSent, RequestTransmissionFailed
 from twisted.web._newclient import ResponseNeverReceived, ResponseFailed
@@ -40,8 +40,8 @@ from twisted.internet.interfaces import IOpenSSLClientConnectionCreator
 from zope.interface.declarations import implementer
 from twisted.web.iweb import IPolicyForHTTPS
 from twisted.python.deprecate import getDeprecationWarningString
-from twisted.python.versions import Version
-from twisted.web.client import BrowserLikePolicyForHTTPS
+from incremental import Version
+from twisted.web.client import (BrowserLikePolicyForHTTPS)
 from twisted.web.error import SchemeNotSupported
 
 try:
@@ -732,6 +732,7 @@ class AgentTests(TestCase, FakeReactorAndConnectMixin, AgentTestsMixin):
     """
     Tests for the new HTTP client API provided by L{Agent}.
     """
+
     def makeAgent(self):
         """
         @return: a new L{twisted.web.client.Agent} instance
@@ -1146,7 +1147,54 @@ class AgentTests(TestCase, FakeReactorAndConnectMixin, AgentTestsMixin):
 
 
 
+<<<<<<< HEAD:twisted/web/test/test_agent.py
 class AgentHTTPSTests(TestCase, FakeReactorAndConnectMixin):
+=======
+class AgentMethodInjectionTests(
+        FakeReactorAndConnectMixin,
+        MethodInjectionTestsMixin,
+        SynchronousTestCase,
+):
+    """
+    Test L{client.Agent} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: see L{MethodInjectionTestsMixin}
+        """
+        agent = client.Agent(self.createReactor())
+        uri = b"http://twisted.invalid"
+        agent.request(method, uri, client.Headers(), None)
+
+
+
+class AgentURIInjectionTests(
+        FakeReactorAndConnectMixin,
+        URIInjectionTestsMixin,
+        SynchronousTestCase,
+):
+    """
+    Test L{client.Agent} against URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided method.
+
+        @param uri: see L{URIInjectionTestsMixin}
+        """
+        agent = client.Agent(self.createReactor())
+        method = b"GET"
+        agent.request(method, uri, client.Headers(), None)
+
+
+
+class AgentHTTPSTests(TestCase, FakeReactorAndConnectMixin,
+                      IntegrationTestingMixin):
+>>>>>>> 6c61fc450... Prevent CRLF injections described in CVE-2019-12387:src/twisted/web/test/test_agent.py
     """
     Tests for the new HTTP client API that depends on SSL.
     """
@@ -2890,3 +2938,199 @@ class ReadBodyTests(TestCase):
 
         warnings = self.flushWarnings()
         self.assertEqual(len(warnings), 0)
+<<<<<<< HEAD:twisted/web/test/test_agent.py
+=======
+
+
+
+class HostnameCachingHTTPSPolicyTests(TestCase):
+
+    skip = skipWhenNoSSL
+
+    def test_cacheIsUsed(self):
+        """
+        Verify that the connection creator is added to the
+        policy's cache, and that it is reused on subsequent calls
+        to creatorForNetLoc.
+
+        """
+        trustRoot = CustomOpenSSLTrustRoot()
+        wrappedPolicy = BrowserLikePolicyForHTTPS(trustRoot=trustRoot)
+        policy = HostnameCachingHTTPSPolicy(wrappedPolicy)
+        creator = policy.creatorForNetloc(b"foo", 1589)
+        self.assertTrue(trustRoot.called)
+        trustRoot.called = False
+        self.assertEquals(1, len(policy._cache))
+        connection = creator.clientConnectionForTLS(None)
+        self.assertIs(trustRoot.context, connection.get_context())
+
+        policy.creatorForNetloc(b"foo", 1589)
+        self.assertFalse(trustRoot.called)
+
+
+    def test_cacheRemovesOldest(self):
+        """
+        Verify that when the cache is full, and a new entry is added,
+        the oldest entry is removed.
+        """
+        trustRoot = CustomOpenSSLTrustRoot()
+        wrappedPolicy = BrowserLikePolicyForHTTPS(trustRoot=trustRoot)
+        policy = HostnameCachingHTTPSPolicy(wrappedPolicy)
+        for i in range(0, 20):
+            hostname = u"host" + unicode(i)
+            policy.creatorForNetloc(hostname.encode("ascii"), 8675)
+
+        # Force host0, which was the first, to be the most recently used
+        host0 = u"host0"
+        policy.creatorForNetloc(host0.encode("ascii"), 309)
+        self.assertIn(host0, policy._cache)
+        self.assertEquals(20, len(policy._cache))
+
+        hostn = u"new"
+        policy.creatorForNetloc(hostn.encode("ascii"), 309)
+
+        host1 = u"host1"
+        self.assertNotIn(host1, policy._cache)
+        self.assertEquals(20, len(policy._cache))
+
+        self.assertIn(hostn, policy._cache)
+        self.assertIn(host0, policy._cache)
+
+        # Accessing an item repeatedly does not corrupt the LRU.
+        for _ in range(20):
+            policy.creatorForNetloc(host0.encode("ascii"), 8675)
+
+        hostNPlus1 = u"new1"
+
+        policy.creatorForNetloc(hostNPlus1.encode("ascii"), 800)
+
+        self.assertNotIn(u"host2", policy._cache)
+        self.assertEquals(20, len(policy._cache))
+
+        self.assertIn(hostNPlus1, policy._cache)
+        self.assertIn(hostn, policy._cache)
+        self.assertIn(host0, policy._cache)
+
+
+    def test_changeCacheSize(self):
+        """
+        Verify that changing the cache size results in a policy that
+        respects the new cache size and not the default.
+
+        """
+        trustRoot = CustomOpenSSLTrustRoot()
+        wrappedPolicy = BrowserLikePolicyForHTTPS(trustRoot=trustRoot)
+        policy = HostnameCachingHTTPSPolicy(wrappedPolicy, cacheSize=5)
+        for i in range(0, 5):
+            hostname = u"host" + unicode(i)
+            policy.creatorForNetloc(hostname.encode("ascii"), 8675)
+
+        first = u"host0"
+        self.assertIn(first, policy._cache)
+        self.assertEquals(5, len(policy._cache))
+
+        hostn = u"new"
+        policy.creatorForNetloc(hostn.encode("ascii"), 309)
+        self.assertNotIn(first, policy._cache)
+        self.assertEquals(5, len(policy._cache))
+
+        self.assertIn(hostn, policy._cache)
+
+
+
+class RequestMethodInjectionTests(
+        MethodInjectionTestsMixin,
+        SynchronousTestCase,
+):
+    """
+    Test L{client.Request} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: see L{MethodInjectionTestsMixin}
+        """
+        client.Request(
+            method=method,
+            uri=b"http://twisted.invalid",
+            headers=http_headers.Headers(),
+            bodyProducer=None,
+        )
+
+
+
+class RequestWriteToMethodInjectionTests(
+        MethodInjectionTestsMixin,
+        SynchronousTestCase,
+):
+    """
+    Test L{client.Request.writeTo} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: see L{MethodInjectionTestsMixin}
+        """
+        headers = http_headers.Headers({b"Host": [b"twisted.invalid"]})
+        req = client.Request(
+            method=b"GET",
+            uri=b"http://twisted.invalid",
+            headers=headers,
+            bodyProducer=None,
+        )
+        req.method = method
+        req.writeTo(StringTransport())
+
+
+
+class RequestURIInjectionTests(
+        URIInjectionTestsMixin,
+        SynchronousTestCase,
+):
+    """
+    Test L{client.Request} against HTTP URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param method: see L{URIInjectionTestsMixin}
+        """
+        client.Request(
+            method=b"GET",
+            uri=uri,
+            headers=http_headers.Headers(),
+            bodyProducer=None,
+        )
+
+
+
+class RequestWriteToURIInjectionTests(
+        URIInjectionTestsMixin,
+        SynchronousTestCase,
+):
+    """
+    Test L{client.Request.writeTo} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided method.
+
+        @param method: see L{URIInjectionTestsMixin}
+        """
+        headers = http_headers.Headers({b"Host": [b"twisted.invalid"]})
+        req = client.Request(
+            method=b"GET",
+            uri=b"http://twisted.invalid",
+            headers=headers,
+            bodyProducer=None,
+        )
+        req.uri = uri
+        req.writeTo(StringTransport())
+>>>>>>> 6c61fc450... Prevent CRLF injections described in CVE-2019-12387:src/twisted/web/test/test_agent.py
diff --git a/twisted/web/test/test_webclient.py b/twisted/web/test/test_webclient.py
index 1627f9408..e1fb0524a 100644
--- a/twisted/web/test/test_webclient.py
+++ b/twisted/web/test/test_webclient.py
@@ -7,6 +7,7 @@ Tests for the old L{twisted.web.client} APIs, C{getPage} and friends.
 
 from __future__ import division, absolute_import
 
+import io
 import os
 from errno import ENOSPC
 
@@ -20,7 +21,8 @@ from twisted.trial import unittest
 from twisted.web import server, client, error, resource
 from twisted.web.static import Data
 from twisted.web.util import Redirect
-from twisted.internet import reactor, defer, interfaces
+from twisted.internet import address, reactor, defer, interfaces
+from twisted.internet.protocol import ClientFactory
 from twisted.python.filepath import FilePath
 from twisted.python.log import msg
 from twisted.protocols.policies import WrappingFactory
@@ -35,6 +37,12 @@ from twisted import test
 serverPEM = FilePath(test.__file__.encode("utf-8")).sibling(b'server.pem')
 serverPEMPath = nativeString(serverPEM.path)
 
+from twisted.web.test.injectionhelpers import (
+    MethodInjectionTestsMixin,
+    URIInjectionTestsMixin,
+)
+
+
 
 _PY3DownloadSkip = "downloadPage will be ported to Python 3 in ticket #6197."
 
@@ -1311,3 +1319,447 @@ class URITests(unittest.TestCase):
         self.assertIsInstance(uri.scheme, bytes)
         self.assertIsInstance(uri.host, bytes)
         self.assertIsInstance(uri.path, bytes)
+
+
+class URITestsForHostname(URITests, unittest.TestCase):
+    """
+    Tests for L{twisted.web.client.URI} with host names.
+    """
+
+    uriHost = host = b"example.com"
+
+
+
+class URITestsForIPv4(URITests, unittest.TestCase):
+    """
+    Tests for L{twisted.web.client.URI} with IPv4 host addresses.
+    """
+
+    uriHost = host = b"192.168.1.67"
+
+
+
+class URITestsForIPv6(URITests, unittest.TestCase):
+    """
+    Tests for L{twisted.web.client.URI} with IPv6 host addresses.
+
+    IPv6 addresses must always be surrounded by square braces in URIs. No
+    attempt is made to test without.
+    """
+
+    host = b"fe80::20c:29ff:fea4:c60"
+    uriHost = b"[fe80::20c:29ff:fea4:c60]"
+
+
+    def test_hostBracketIPv6AddressLiteral(self):
+        """
+        Brackets around IPv6 addresses are stripped in the host field. The host
+        field is then exported with brackets in the output of
+        L{client.URI.toBytes}.
+        """
+        uri = client.URI.fromBytes(b"http://[::1]:80/index.html")
+
+        self.assertEqual(uri.host, b"::1")
+        self.assertEqual(uri.netloc, b"[::1]:80")
+        self.assertEqual(uri.toBytes(), b'http://[::1]:80/index.html')
+
+
+
+class DeprecationTests(unittest.TestCase):
+    """
+    Tests that L{client.getPage} and friends are deprecated.
+    """
+
+    def test_getPageDeprecated(self):
+        """
+        L{client.getPage} is deprecated.
+        """
+        port = reactor.listenTCP(
+            0, server.Site(Data(b'', 'text/plain')), interface="127.0.0.1")
+        portno = port.getHost().port
+        self.addCleanup(port.stopListening)
+        url = networkString("http://127.0.0.1:%d" % (portno,))
+
+        d = client.getPage(url)
+        warningInfo = self.flushWarnings([self.test_getPageDeprecated])
+        self.assertEqual(len(warningInfo), 1)
+        self.assertEqual(warningInfo[0]['category'], DeprecationWarning)
+        self.assertEqual(
+            warningInfo[0]['message'],
+            "twisted.web.client.getPage was deprecated in "
+            "Twisted 16.7.0; please use https://pypi.org/project/treq/ or twisted.web.client.Agent instead")
+
+        return d.addErrback(lambda _: None)
+
+
+    def test_downloadPageDeprecated(self):
+        """
+        L{client.downloadPage} is deprecated.
+        """
+        port = reactor.listenTCP(
+            0, server.Site(Data(b'', 'text/plain')), interface="127.0.0.1")
+        portno = port.getHost().port
+        self.addCleanup(port.stopListening)
+        url = networkString("http://127.0.0.1:%d" % (portno,))
+
+        path = FilePath(self.mktemp())
+        d = client.downloadPage(url, path.path)
+
+        warningInfo = self.flushWarnings([self.test_downloadPageDeprecated])
+        self.assertEqual(len(warningInfo), 1)
+        self.assertEqual(warningInfo[0]['category'], DeprecationWarning)
+        self.assertEqual(
+            warningInfo[0]['message'],
+            "twisted.web.client.downloadPage was deprecated in "
+            "Twisted 16.7.0; please use https://pypi.org/project/treq/ or twisted.web.client.Agent instead")
+
+        return d.addErrback(lambda _: None)
+
+
+    def _testDeprecatedClass(self, klass):
+        """
+        Assert that accessing the given class was deprecated.
+
+        @param klass: The class being deprecated.
+        @type klass: L{str}
+        """
+        getattr(client, klass)
+
+        warningInfo = self.flushWarnings()
+        self.assertEqual(len(warningInfo), 1)
+        self.assertEqual(warningInfo[0]['category'], DeprecationWarning)
+        self.assertEqual(
+            warningInfo[0]['message'],
+            "twisted.web.client.{} was deprecated in "
+            "Twisted 16.7.0: please use https://pypi.org/project/treq/ or twisted.web.client.Agent instead".format(klass))
+
+
+    def test_httpPageGetterDeprecated(self):
+        """
+        L{client.HTTPPageGetter} is deprecated.
+        """
+        self._testDeprecatedClass("HTTPPageGetter")
+
+
+    def test_httpPageDownloaderDeprecated(self):
+        """
+        L{client.HTTPPageDownloader} is deprecated.
+        """
+        self._testDeprecatedClass("HTTPPageDownloader")
+
+
+    def test_httpClientFactoryDeprecated(self):
+        """
+        L{client.HTTPClientFactory} is deprecated.
+        """
+        self._testDeprecatedClass("HTTPClientFactory")
+
+
+    def test_httpDownloaderDeprecated(self):
+        """
+        L{client.HTTPDownloader} is deprecated.
+        """
+        self._testDeprecatedClass("HTTPDownloader")
+
+
+
+class GetPageMethodInjectionTests(
+        MethodInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Test L{client.getPage} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: see L{MethodInjectionTestsMixin}
+        """
+        uri = b'http://twisted.invalid'
+        client.getPage(uri, method=method)
+
+
+
+class GetPageURIInjectionTests(
+        URIInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Test L{client.getPage} against URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param uri: see L{URIInjectionTestsMixin}
+        """
+        client.getPage(uri)
+
+
+
+class DownloadPageMethodInjectionTests(
+        MethodInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Test L{client.getPage} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: see L{MethodInjectionTestsMixin}
+        """
+        uri = b'http://twisted.invalid'
+        client.downloadPage(uri, file=io.BytesIO(), method=method)
+
+
+
+class DownloadPageURIInjectionTests(
+        URIInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Test L{client.downloadPage} against URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param uri: see L{URIInjectionTestsMixin}
+        """
+        client.downloadPage(uri, file=io.BytesIO())
+
+
+
+def makeHTTPPageGetterFactory(protocolClass, method, host, path):
+    """
+    Make a L{ClientFactory} that can be used with
+    L{client.HTTPPageGetter} and its subclasses.
+
+    @param protocolClass: The protocol class
+    @type protocolClass: A subclass of L{client.HTTPPageGetter}
+
+    @param method: the HTTP method
+
+    @param host: the host
+
+    @param path: The URI path
+
+    @return: A L{ClientFactory}.
+    """
+    factory = ClientFactory.forProtocol(protocolClass)
+
+    factory.method = method
+    factory.host = host
+    factory.path = path
+
+    factory.scheme = b"http"
+    factory.port = 0
+    factory.headers = {}
+    factory.agent = b"User/Agent"
+    factory.cookies = {}
+
+    return factory
+
+
+
+class HTTPPageGetterMethodInjectionTests(
+        MethodInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Test L{client.HTTPPageGetter} against HTTP method injections.
+    """
+    protocolClass = client.HTTPPageGetter
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: L{MethodInjectionTestsMixin}
+        """
+        transport = StringTransport()
+        factory = makeHTTPPageGetterFactory(
+            self.protocolClass,
+            method=method,
+            host=b"twisted.invalid",
+            path=b"/",
+        )
+        getter = factory.buildProtocol(
+            address.IPv4Address("TCP", "127.0.0.1", 0),
+        )
+        getter.makeConnection(transport)
+
+
+
+class HTTPPageGetterURIInjectionTests(
+        URIInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Test L{client.HTTPPageGetter} against HTTP URI injections.
+    """
+    protocolClass = client.HTTPPageGetter
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param uri: L{URIInjectionTestsMixin}
+        """
+        transport = StringTransport()
+        # Setting the host and path to the same value is imprecise but
+        # doesn't require parsing an invalid URI.
+        factory = makeHTTPPageGetterFactory(
+            self.protocolClass,
+            method=b"GET",
+            host=uri,
+            path=uri,
+        )
+        getter = factory.buildProtocol(
+            address.IPv4Address("TCP", "127.0.0.1", 0),
+        )
+        getter.makeConnection(transport)
+
+
+
+class HTTPPageDownloaderMethodInjectionTests(
+        HTTPPageGetterMethodInjectionTests
+):
+
+    """
+    Test L{client.HTTPPageDownloader} against HTTP method injections.
+    """
+    protocolClass = client.HTTPPageDownloader
+
+
+
+class HTTPPageDownloaderURIInjectionTests(
+        HTTPPageGetterURIInjectionTests
+):
+    """
+    Test L{client.HTTPPageDownloader} against HTTP URI injections.
+    """
+    protocolClass = client.HTTPPageDownloader
+
+
+
+class HTTPClientFactoryMethodInjectionTests(
+        MethodInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Tests L{client.HTTPClientFactory} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: L{MethodInjectionTestsMixin}
+        """
+        client.HTTPClientFactory(b"https://twisted.invalid", method)
+
+
+
+class HTTPClientFactoryURIInjectionTests(
+        URIInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Tests L{client.HTTPClientFactory} against HTTP URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param uri: L{URIInjectionTestsMixin}
+        """
+        client.HTTPClientFactory(uri)
+
+
+
+class HTTPClientFactorySetURLURIInjectionTests(
+        URIInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Tests L{client.HTTPClientFactory.setURL} against HTTP URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param uri: L{URIInjectionTestsMixin}
+        """
+        client.HTTPClientFactory(b"https://twisted.invalid").setURL(uri)
+
+
+
+class HTTPDownloaderMethodInjectionTests(
+        MethodInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Tests L{client.HTTPDownloader} against HTTP method injections.
+    """
+
+    def attemptRequestWithMaliciousMethod(self, method):
+        """
+        Attempt a request with the provided method.
+
+        @param method: L{MethodInjectionTestsMixin}
+        """
+        client.HTTPDownloader(
+            b"https://twisted.invalid",
+            io.BytesIO(),
+            method=method,
+        )
+
+
+
+class HTTPDownloaderURIInjectionTests(
+        URIInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Tests L{client.HTTPDownloader} against HTTP URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param uri: L{URIInjectionTestsMixin}
+        """
+        client.HTTPDownloader(uri, io.BytesIO())
+
+
+
+class HTTPDownloaderSetURLURIInjectionTests(
+        URIInjectionTestsMixin,
+        unittest.SynchronousTestCase,
+):
+    """
+    Tests L{client.HTTPDownloader.setURL} against HTTP URI injections.
+    """
+
+    def attemptRequestWithMaliciousURI(self, uri):
+        """
+        Attempt a request with the provided URI.
+
+        @param uri: L{URIInjectionTestsMixin}
+        """
+        downloader = client.HTTPDownloader(
+            b"https://twisted.invalid",
+            io.BytesIO(),
+        )
+        downloader.setURL(uri)
-- 
2.21.0

Places

File 0001-Prevent-CRLF-injections-described-in-CVE-2019-12387.patch of Package python-Twisted (Revision 3c3db51246bcf79f688f0f5b4df615b5)

Places