CVE-2020-26137: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: urllib3 before 1.25.9 allows CRLF injection if the attacker controls the HTTP request method, as demonstrated by inserting CR and LF control characters in the first argument of putrequest(). NOTE: this is similar to CVE-2020-26116.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 6.5
Published: 2020-09-30T18:15Z
Repository: https://github.com/urllib3/urllib3
Language: Python
Files changed: CHANGES.rst, src/urllib3/connection.py, test/with_dummyserver/test_connectionpool.py
Commit date: 2020-02-17 15:34:48 -0600
Commit message: Raise ValueError if method contains control characters (#1800)


Diff: diff --git a/CHANGES.rst b/CHANGES.rst
index dbbf6949..888b777e 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,8 +1,15 @@
 Changes
 =======
 
+master (dev)
+------------
+
+* Raise ``ValueError`` if control characters are given in
+  the ``method`` parameter of ``HTTPConnection.request()`` (Pull #1800)
+
+
 1.25.8 (2020-01-20)
 -------------------
 
 * Drop support for EOL Python 3.4 (Pull #1774)
 
diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py
index 71e6790b..f7b17609 100644
--- a/src/urllib3/connection.py
+++ b/src/urllib3/connection.py
@@ -1,6 +1,7 @@
 from __future__ import absolute_import
+import re
 import datetime
 import logging
 import os
 import socket
 from socket import error as SocketError, timeout as SocketTimeout
@@ -56,10 +57,12 @@ port_by_scheme = {"http": 80, "https": 443}
 
 # When it comes time to update this value as a part of regular maintenance
 # (ie test_recent_date is failing) update it to ~6 months before the current date.
 RECENT_DATE = datetime.date(2019, 1, 1)
 
+_CONTAINS_CONTROL_CHAR_RE = re.compile(r"[^-!#$%&'*+.^_`|~0-9a-zA-Z]")
+
 
 class DummyConnection(object):
     """Used to detect a failed ConnectionCls import."""
 
     pass
@@ -182,10 +185,21 @@ class HTTPConnection(_HTTPConnection, object):
 
     def connect(self):
         conn = self._new_conn()
         self._prepare_conn(conn)
 
+    def putrequest(self, method, url, *args, **kwargs):
+        """Send a request to the server"""
+        match = _CONTAINS_CONTROL_CHAR_RE.search(method)
+        if match:
+            raise ValueError(
+                "Method cannot contain non-token characters %r (found at least %r)"
+                % (method, match.group())
+            )
+
+        return _HTTPConnection.putrequest(self, method, url, *args, **kwargs)
+
     def request_chunked(self, method, url, body=None, headers=None):
         """
         Alternative to the common request method, which sends the
         body with chunked encoding and not as one block
         """
diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py
index 57f0dbd2..79cbd271 100644
--- a/test/with_dummyserver/test_connectionpool.py
+++ b/test/with_dummyserver/test_connectionpool.py
@@ -675,10 +675,16 @@ class TestConnectionPool(HTTPDummyServerTestCase):
             "thishostdoesnotexist.invalid", self.port, timeout=0.001
         ) as pool:
             with pytest.raises(MaxRetryError):
                 pool.request("GET", "/test", retries=2)
 
+    @pytest.mark.parametrize("char", [" ", "\r", "\n", "\x00"])
+    def test_invalid_method_not_allowed(self, char):
+        with pytest.raises(ValueError):
+            with HTTPConnectionPool(self.host, self.port) as pool:
+                pool.request("GET" + char, "/")
+
     def test_percent_encode_invalid_target_chars(self):
         with HTTPConnectionPool(self.host, self.port) as pool:
             r = pool.request("GET", "/echo_params?q=\r&k=\n \n")
             assert r.data == b"[('k', '\\n \\n'), ('q', '\\r')]"
 

Fixed code: master (dev)
------------

* Raise ``ValueError`` if control characters are given in
  the ``method`` parameter of ``HTTPConnection.request()`` (Pull #1800)


import re
_CONTAINS_CONTROL_CHAR_RE = re.compile(r"[^-!#$%&'*+.^_`|~0-9a-zA-Z]")

    def putrequest(self, method, url, *args, **kwargs):
        """Send a request to the server"""
        match = _CONTAINS_CONTROL_CHAR_RE.search(method)
        if match:
            raise ValueError(
                "Method cannot contain non-token characters %r (found at least %r)"
                % (method, match.group())
            )

        return _HTTPConnection.putrequest(self, method, url, *args, **kwargs)

    @pytest.mark.parametrize("char", [" ", "\r", "\n", "\x00"])
    def test_invalid_method_not_allowed(self, char):
        with pytest.raises(ValueError):
            with HTTPConnectionPool(self.host, self.port) as pool:
                pool.request("GET" + char, "/")