File 0001-Prevent-CRLF-injections-described-in-CVE-2019-12387.patch of Package python-Twisted.11555
From 1eebdcc797fe35cfcd44fe5361af606f1a080979 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)
---
src/twisted/web/_newclient.py | 85 +++++-
src/twisted/web/client.py | 25 +-
src/twisted/web/newsfragments/9647.bugfix | 1 +
src/twisted/web/test/injectionhelpers.py | 168 ++++++++++++
src/twisted/web/test/test_agent.py | 241 ++++++++++++++++-
src/twisted/web/test/test_webclient.py | 316 +++++++++++++++++++++-
6 files changed, 825 insertions(+), 11 deletions(-)
create mode 100644 src/twisted/web/newsfragments/9647.bugfix
create mode 100644 src/twisted/web/test/injectionhelpers.py
diff --git a/src/twisted/web/_newclient.py b/src/twisted/web/_newclient.py
index d206dc351..9cca3ce91 100644
--- a/src/twisted/web/_newclient.py
+++ b/src/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
@@ -574,6 +576,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:
"""
@@ -612,8 +682,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
@@ -658,8 +728,15 @@ class Request:
# method would probably be good. It would be nice if this method
# weren't limited to issuing 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/src/twisted/web/client.py b/src/twisted/web/client.py
index a4a942dc7..47494589a 100644
--- a/src/twisted/web/client.py
+++ b/src/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:
@@ -361,7 +366,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)
@@ -388,6 +393,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:
@@ -731,7 +737,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
@@ -1419,6 +1425,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()
@@ -1644,6 +1656,7 @@ class Agent(_AgentBase):
@see: L{twisted.web.iweb.IAgent.request}
"""
+ uri = _ensureValidURI(uri.strip())
parsedURI = URI.fromBytes(uri)
try:
endpoint = self._getEndpoint(parsedURI)
@@ -1677,6 +1690,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/src/twisted/web/newsfragments/9647.bugfix b/src/twisted/web/newsfragments/9647.bugfix
new file mode 100644
index 000000000..b76916cdc
--- /dev/null
+++ b/src/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/src/twisted/web/test/injectionhelpers.py b/src/twisted/web/test/injectionhelpers.py
new file mode 100644
index 000000000..ffeb8629c
--- /dev/null
+++ b/src/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/src/twisted/web/test/test_agent.py b/src/twisted/web/test/test_agent.py
index a549b5ba5..2ac0b3786 100644
--- a/src/twisted/web/test/test_agent.py
+++ b/src/twisted/web/test/test_agent.py
@@ -11,7 +11,7 @@ from io import BytesIO
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
@@ -49,6 +49,10 @@ from twisted.internet.endpoints import HostnameEndpoint
from twisted.test.proto_helpers import AccumulatingProtocol
from twisted.test.iosim import IOPump, FakeTransport
from twisted.test.test_sslverify import certificatesForAuthorityAndServer
+from twisted.web.test.injectionhelpers import (
+ MethodInjectionTestsMixin,
+ URIInjectionTestsMixin,
+)
from twisted.web.error import SchemeNotSupported
try:
@@ -856,6 +860,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
@@ -1277,6 +1282,48 @@ class AgentTests(TestCase, FakeReactorAndConnectMixin, AgentTestsMixin,
+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):
"""
@@ -3054,3 +3101,195 @@ class ReadBodyTests(TestCase):
warnings = self.flushWarnings()
self.assertEqual(len(warnings), 0)
+
+
+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())
diff --git a/src/twisted/web/test/test_webclient.py b/src/twisted/web/test/test_webclient.py
index f978b153c..05a4c1f8e 100644
--- a/src/twisted/web/test/test_webclient.py
+++ b/src/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, util
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
@@ -33,6 +35,15 @@ except:
ssl = None
from twisted import test
+from twisted.logger import (globalLogPublisher, FilteringLogObserver,
+ LogLevelFilterPredicate, LogLevel, Logger)
+
+from twisted.web.test.injectionhelpers import (
+ MethodInjectionTestsMixin,
+ URIInjectionTestsMixin,
+)
+
+
serverPEM = FilePath(test.__file__).sibling('server.pem')
serverPEMPath = serverPEM.asBytesMode().path
@@ -1494,3 +1505,306 @@ class DeprecationTests(unittest.TestCase):
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
