Merge ~andrey-fedoseev/twisted:10323-http-request-smuggling into ~launchpad/twisted:lp-backport

[Andrey Fedoseev]

10323: Inconsistent Interpretation of HTTP Requests ('HTTP Request Smuggling') in twisted.web

[Jürgen Gmach (jugmac00)]

@Colin: Do we have a common procedure for backports? e.g. mark them with a comment?

Any opinion on the cherry-pick? Took me a min to deduct that 10323 is GitHub's issue number in the Twisted project.

[Colin Watson (cjwatson)]

The substance of the changes here looks OK to me, thanks. We don't need to mark backports with comments - that's what git history is for.

However, please amend the git commit message with a reference to https://github.com/twisted/twisted/issues/10323 (and perhaps a description of anything not entirely straightforward about the backport, since it doesn't quite look like a straight cherry-pick of the commit referenced in the upstream issue?) before landing this, so that people don't have to try to deduce it themselves if necessary.

review: Approve

[Jürgen Gmach (jugmac00)]

@Colin - how would you find all backports quickly if you do not use a marker - only with git commits? This might be handy once we try to use upstream again - but maybe this info is not even necessary. Let's hope we can just switch to upstream at some point and rely on the test suite to guide us.

[Colin Watson (cjwatson)]

"Find all backports quickly" isn't a use case I ever need. When rebasing on a new upstream, git commits are exactly the right tool.

[Colin Watson (cjwatson)]

Also, adding markers in the code makes things actively *worse* when you're trying to rebase on a new upstream, because now git can't do its job properly and help you: instead of being able to automatically drop patches which are already upstream, everything will conflict. So I really strongly recommend against that.

[Andrey Fedoseev (andrey-fedoseev)]

@Colin

I've updated the commit message.

diff --git a/src/twisted/_version.py b/src/twisted/_version.py
index 05d17b9..68213c3 100644
--- a/src/twisted/_version.py
+++ b/src/twisted/_version.py
@@ -24,5 +24,5 @@ class LocalVersion(Version):
     local = public
 
 
-__version__ = LocalVersion('Twisted', 20, 3, 0, local='+lp8')
+__version__ = LocalVersion('Twisted', 20, 3, 0, local='+lp9')
 __all__ = ["__version__"]
diff --git a/src/twisted/web/http.py b/src/twisted/web/http.py
index b7afa8b..ffe970b 100644
--- a/src/twisted/web/http.py
+++ b/src/twisted/web/http.py
@@ -340,10 +340,39 @@ def toChunk(data):
 
 
 
+def _ishexdigits(b):
+    """
+    Is the string case-insensitively hexidecimal?
+
+    It must be composed of one or more characters in the ranges a-f, A-F
+    and 0-9.
+    """
+    for c in b:
+        if c not in b"0123456789abcdefABCDEF":
+            return False
+    return b != b""
+
+
+def _hexint(b):
+    """
+    Decode a hexadecimal integer.
+
+    Unlike L{int(b, 16)}, this raises L{ValueError} when the integer has
+    a prefix like C{b'0x'}, C{b'+'}, or C{b'-'}, which is desirable when
+    parsing network protocols.
+    """
+    if not _ishexdigits(b):
+        raise ValueError(b)
+    return int(b, 16)
+
+
 def fromChunk(data):
     """
     Convert chunk to string.
 
+    Note that this function is not specification compliant: it doesn't handle
+    chunk extensions.
+
     @type data: C{bytes}
 
     @return: tuple of (result, remaining) - both C{bytes}.
@@ -352,7 +381,7 @@ def fromChunk(data):
         byte string.
     """
     prefix, rest = data.split(b'\r\n', 1)
-    length = int(prefix, 16)
+    length = _hexint(prefix)
     if length < 0:
         raise ValueError("Chunk length must be >= 0, not %d" % (length,))
     if rest[length:length + 2] != b'\r\n':
@@ -1775,11 +1804,54 @@ class _IdentityTransferDecoder(object):
             raise _DataLoss()
 
 
+maxChunkSizeLineLength = 1024
+
+
+_chunkExtChars = (
+    b"\t !\"#$%&'()*+,-./0123456789:;<=>?@"
+    b"ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`"
+    b"abcdefghijklmnopqrstuvwxyz{|}~"
+    b"\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f"
+    b"\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f"
+    b"\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf"
+    b"\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf"
+    b"\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf"
+    b"\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf"
+    b"\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef"
+    b"\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff"
+)
+"""
+Characters that are valid in a chunk extension.
+
+See RFC 7230 section 4.1.1::
+
+     chunk-ext      = *( ";" chunk-ext-name [ "=" chunk-ext-val ] )
+
+     chunk-ext-name = token
+     chunk-ext-val  = token / quoted-string
+
+And section 3.2.6::
+
+     token          = 1*tchar
+
+     tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
+                    / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
+                    / DIGIT / ALPHA
+                    ; any VCHAR, except delimiters
+
+     quoted-string  = DQUOTE *( qdtext / quoted-pair ) DQUOTE
+     qdtext         = HTAB / SP /%x21 / %x23-5B / %x5D-7E / obs-text
+     obs-text       = %x80-FF
+
+We don't check if chunk extensions are well-formed beyond validating that they
+don't contain characters outside this range.
+"""
+
 
 class _ChunkedTransferDecoder(object):
     """
-    Protocol for decoding I{chunked} Transfer-Encoding, as defined by RFC 2616,
-    section 3.6.1.  This protocol can interpret the contents of a request or
+    Protocol for decoding I{chunked} Transfer-Encoding, as defined by RFC 7230,
+    section 4.1.  This protocol can interpret the contents of a request or
     response body which uses the I{chunked} Transfer-Encoding.  It cannot
     interpret any of the rest of the HTTP protocol.
 
@@ -1795,7 +1867,7 @@ class _ChunkedTransferDecoder(object):
     noticed. -exarkun
 
     @ivar dataCallback: A one-argument callable which will be invoked each
-        time application data is received.
+        time application data is received. This callback is not reentrant.
 
     @ivar finishCallback: A one-argument callable which will be invoked when
         the terminal chunk is received.  It will be invoked with all bytes
@@ -1813,81 +1885,162 @@ class _ChunkedTransferDecoder(object):
         read. For C{'BODY'}, the contents of a chunk are being read. For
         C{'FINISHED'}, the last chunk has been completely read and no more
         input is valid.
+
+    @ivar _buffer: Accumulated received data for the current state. At each
+        state transition this is truncated at the front so that index 0 is
+        where the next state shall begin.
+
+    @ivar _start: While in the C{'CHUNK_LENGTH'} state, tracks the index into
+        the buffer at which search for CRLF should resume. Resuming the search
+        at this position avoids doing quadratic work if the chunk length line
+        arrives over many calls to C{dataReceived}.
+
+        Not used in any other state.
     """
+
     state = 'CHUNK_LENGTH'
 
     def __init__(self, dataCallback, finishCallback):
         self.dataCallback = dataCallback
         self.finishCallback = finishCallback
-        self._buffer = b''
+        self._buffer = bytearray()
+        self._start = 0
+
+    def _dataReceived_CHUNK_LENGTH(self):
+        """
+        Read the chunk size line, ignoring any extensions.
 
+        @returns: C{True} once the line has been read and removed from
+            C{self._buffer}.  C{False} when more data is required.
 
-    def _dataReceived_CHUNK_LENGTH(self, data):
-        if b'\r\n' in data:
-            line, rest = data.split(b'\r\n', 1)
-            parts = line.split(b';')
-            try:
-                self.length = int(parts[0], 16)
-            except ValueError:
-                raise _MalformedChunkedDataError(
-                    "Chunk-size must be an integer.")
-            if self.length == 0:
-                self.state = 'TRAILER'
-            else:
-                self.state = 'BODY'
-            return rest
-        else:
-            self._buffer = data
-            return b''
+        @raises _MalformedChunkedDataError: when the chunk size cannot be
+            decoded or the length of the line exceeds L{maxChunkSizeLineLength}.
+        """
+        eolIndex = self._buffer.find(b'\r\n', self._start)
 
+        if eolIndex >= maxChunkSizeLineLength or (
+            eolIndex == -1 and len(self._buffer) > maxChunkSizeLineLength
+        ):
+            raise _MalformedChunkedDataError(
+                "Chunk size line exceeds maximum of {} bytes.".format(
+                    maxChunkSizeLineLength
+                )
+            )
 
-    def _dataReceived_CRLF(self, data):
-        if data.startswith(b'\r\n'):
-            self.state = 'CHUNK_LENGTH'
-            return data[2:]
-        else:
-            self._buffer = data
-            return b''
+        if eolIndex == -1:
+            # Restart the search upon receipt of more data at the start of the
+            # new data, minus one in case the last character of the buffer is
+            # CR.
+            self._start = len(self._buffer) - 1
+            return False
+
+        endOfLengthIndex = self._buffer.find(b';', 0, eolIndex)
+        if endOfLengthIndex == -1:
+            endOfLengthIndex = eolIndex
+        rawLength = self._buffer[0:endOfLengthIndex]
+        try:
+            length = _hexint(rawLength)
+        except ValueError:
+            raise _MalformedChunkedDataError("Chunk-size must be an integer.")
 
+        ext = self._buffer[endOfLengthIndex + 1 : eolIndex]
+        if ext and ext.translate(None, _chunkExtChars) != b'':
+            raise _MalformedChunkedDataError(
+                "Invalid characters in chunk extensions: {}.".format(ext)
+            )
 
-    def _dataReceived_TRAILER(self, data):
-        if data.startswith(b'\r\n'):
-            data = data[2:]
-            self.state = 'FINISHED'
-            self.finishCallback(data)
+        if length == 0:
+            self.state = 'TRAILER'
         else:
-            self._buffer = data
-        return b''
+            self.state = 'BODY'
 
+        self.length = length
+        del self._buffer[0 : eolIndex + 2]
+        self._start = 0
+        return True
 
-    def _dataReceived_BODY(self, data):
-        if len(data) >= self.length:
-            chunk, data = data[:self.length], data[self.length:]
-            self.dataCallback(chunk)
-            self.state = 'CRLF'
-            return data
-        elif len(data) < self.length:
-            self.length -= len(data)
-            self.dataCallback(data)
-            return b''
+    def _dataReceived_CRLF(self):
+        """
+        Await the carriage return and line feed characters that are the end of
+        chunk marker that follow the chunk data.
 
+        @returns: C{True} when the CRLF have been read, otherwise C{False}.
 
-    def _dataReceived_FINISHED(self, data):
+        @raises _MalformedChunkedDataError: when anything other than CRLF are
+            received.
+        """
+        if len(self._buffer) < 2:
+            return False
+
+        if not self._buffer.startswith(b'\r\n'):
+            raise _MalformedChunkedDataError("Chunk did not end with CRLF")
+
+        self.state = 'CHUNK_LENGTH'
+        del self._buffer[0:2]
+        return True
+
+    def _dataReceived_TRAILER(self):
+        """
+        Await the carriage return and line feed characters that follow the
+        terminal zero-length chunk. Then invoke C{finishCallback} and switch to
+        state C{'FINISHED'}.
+
+        @returns: C{False}, as there is either insufficient data to continue,
+            or no data remains.
+
+        @raises _MalformedChunkedDataError: when anything other than CRLF is
+            received.
+        """
+        if len(self._buffer) < 2:
+            return False
+
+        if not self._buffer.startswith(b'\r\n'):
+            raise _MalformedChunkedDataError("Chunk did not end with CRLF")
+
+        data = memoryview(self._buffer)[2:].tobytes()
+        del self._buffer[:]
+        self.state = 'FINISHED'
+        self.finishCallback(data)
+        return False
+
+    def _dataReceived_BODY(self):
+        """
+        Deliver any available chunk data to the C{dataCallback}. When all the
+        remaining data for the chunk arrives, switch to state C{'CRLF'}.
+
+        @returns: C{True} to continue processing of any buffered data.
+        """
+        if len(self._buffer) >= self.length:
+            chunk = memoryview(self._buffer)[: self.length].tobytes()
+            del self._buffer[: self.length]
+            self.state = 'CRLF'
+            self.dataCallback(chunk)
+        else:
+            chunk = bytes(self._buffer)
+            self.length -= len(chunk)
+            del self._buffer[:]
+            self.dataCallback(chunk)
+        return True
+
+    def _dataReceived_FINISHED(self):
+        """
+        Once C{finishCallback} has been invoked receipt of additional data
+        raises L{RuntimeError} because it represents a programming error in
+        the caller.
+        """
         raise RuntimeError(
             "_ChunkedTransferDecoder.dataReceived called after last "
             "chunk was processed")
 
-
     def dataReceived(self, data):
         """
         Interpret data from a request or response body which uses the
         I{chunked} Transfer-Encoding.
         """
-        data = self._buffer + data
-        self._buffer = b''
-        while data:
-            data = getattr(self, '_dataReceived_%s' % (self.state,))(data)
-
+        self._buffer += data
+        goOn = True
+        while goOn and self._buffer:
+            goOn = getattr(self, '_dataReceived_' + self.state)()
 
     def noMoreData(self):
         """
@@ -1900,7 +2053,6 @@ class _ChunkedTransferDecoder(object):
                 "get to 'FINISHED' state." % (self.state,))
 
 
-
 @implementer(interfaces.IPushProducer)
 class _NoPushProducer(object):
     """
@@ -2157,7 +2309,7 @@ class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
                 self.setRawMode()
         elif line[0] in b' \t':
             # Continuation of a multi line header.
-            self.__header = self.__header + b'\n' + line
+            self.__header += b' ' + line.lstrip(b' \t')
         # Regular header line.
         # Processing of header line is delayed to allow accumulating multi
         # line headers.
@@ -2186,6 +2338,8 @@ class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
 
         # Can this header determine the length?
         if header == b'content-length':
+            if not data.isdigit():
+                return fail()
             try:
                 length = int(data)
             except ValueError:
@@ -2240,7 +2394,7 @@ class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
             return False
 
         header = header.lower()
-        data = data.strip()
+        data = data.strip(b' \t')
 
         if not self._maybeChooseTransferDecoder(header, data):
             return False
diff --git a/src/twisted/web/newsfragments/10323.bugfix b/src/twisted/web/newsfragments/10323.bugfix
new file mode 100644
index 0000000..1890b1b
--- /dev/null
+++ b/src/twisted/web/newsfragments/10323.bugfix
@@ -0,0 +1 @@
+twisted.web.http had several several defects in HTTP request parsing that could permit HTTP request smuggling. It now disallows signed Content-Length headers, forbids illegal characters in chunked extensions, forbids 0x prefix to chunk lengths, and only strips spaces and horizontal tab characters from header values. These changes address CVE-2022-24801 and GHSA-c2jg-hw38-jrqq.
diff --git a/src/twisted/web/test/test_http.py b/src/twisted/web/test/test_http.py
index a3067f7..f50fabe 100644
--- a/src/twisted/web/test/test_http.py
+++ b/src/twisted/web/test/test_http.py
@@ -1300,6 +1300,126 @@ class ChunkedTransferEncodingTests(unittest.TestCase):
         p.dataReceived(b'3; x-foo=bar\r\nabc\r\n')
         self.assertEqual(L, [b'abc'])
 
+    def test_extensionsMalformed(self):
+        """
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the chunk extension fields contain
+        invalid characters.
+
+        This is a potential request smuggling vector: see GHSA-c2jg-hw38-jrqq.
+        """
+        invalidControl = (
+            b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\n\x0b\x0c\r\x0e\x0f'
+            b'\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f'
+        )
+        invalidDelimiter = b'\\'
+        invalidDel = b'\x7f'
+        for b in invalidControl + invalidDelimiter + invalidDel:
+            data = b'3; ' + bytes((b,)) + b'\r\nabc\r\n'
+            p = http._ChunkedTransferDecoder(
+                lambda b: None,  # pragma: nocov
+                lambda b: None,  # pragma: nocov
+            )
+            self.assertRaises(http._MalformedChunkedDataError, p.dataReceived, data)
+
+    def test_oversizedChunkSizeLine(self):
+        """
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the chunk size line exceeds 4 KiB.
+        This applies even when the data has already been received and buffered
+        so that behavior is consistent regardless of how bytes are framed.
+        """
+        p = http._ChunkedTransferDecoder(None, None)
+        self.assertRaises(
+            http._MalformedChunkedDataError,
+            p.dataReceived,
+            b"3;" + b"." * http.maxChunkSizeLineLength + b'\r\nabc\r\n',
+        )
+
+    def test_oversizedChunkSizeLinePartial(self):
+        """
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the amount of data buffered while
+        looking for the end of the chunk size line exceeds 4 KiB so
+        that buffering does not continue without bound.
+        """
+        p = http._ChunkedTransferDecoder(None, None)
+        self.assertRaises(
+            http._MalformedChunkedDataError,
+            p.dataReceived,
+            b"." * (http.maxChunkSizeLineLength + 1),
+        )
+
+    def test_malformedChunkSize(self):
+        """
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the chunk size can't be decoded as
+        a base-16 integer.
+        """
+        p = http._ChunkedTransferDecoder(
+            lambda b: None,  # pragma: nocov
+            lambda b: None,  # pragma: nocov
+        )
+        self.assertRaises(
+            http._MalformedChunkedDataError, p.dataReceived, b'bloop\r\nabc\r\n'
+        )
+
+    def test_malformedChunkSizeNegative(self):
+        """
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the chunk size is negative.
+        """
+        p = http._ChunkedTransferDecoder(
+            lambda b: None,  # pragma: nocov
+            lambda b: None,  # pragma: nocov
+        )
+        self.assertRaises(
+            http._MalformedChunkedDataError, p.dataReceived, b'-3\r\nabc\r\n'
+        )
+
+    def test_malformedChunkSizeHex(self):
+        """
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the chunk size is prefixed with
+        "0x", as if it were a Python integer literal.
+
+        This is a potential request smuggling vector: see GHSA-c2jg-hw38-jrqq.
+        """
+        p = http._ChunkedTransferDecoder(
+            lambda b: None,  # pragma: nocov
+            lambda b: None,  # pragma: nocov
+        )
+        self.assertRaises(
+            http._MalformedChunkedDataError, p.dataReceived, b'0x3\r\nabc\r\n'
+        )
+
+    def test_malformedChunkEnd(self):
+        r"""
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the chunk is followed by characters
+        other than C{\r\n}.
+        """
+        p = http._ChunkedTransferDecoder(
+            lambda b: None,
+            lambda b: None,  # pragma: nocov
+        )
+        self.assertRaises(
+            http._MalformedChunkedDataError, p.dataReceived, b'3\r\nabc!!!!'
+        )
+
+    def test_malformedChunkEndFinal(self):
+        r"""
+        L{_ChunkedTransferDecoder.dataReceived} raises
+        L{_MalformedChunkedDataError} when the terminal zero-length chunk is
+        followed by characters other than C{\r\n}.
+        """
+        p = http._ChunkedTransferDecoder(
+            lambda b: None,
+            lambda b: None,  # pragma: nocov
+        )
+        self.assertRaises(
+            http._MalformedChunkedDataError, p.dataReceived, b'3\r\nabc\r\n0\r\n!!'
+        )
 
     def test_finish(self):
         """
@@ -1389,6 +1509,8 @@ class ChunkingTests(unittest.TestCase, ResponseTestMixin):
             chunked = b''.join(http.toChunk(s))
             self.assertEqual((s, b''), http.fromChunk(chunked))
         self.assertRaises(ValueError, http.fromChunk, b'-5\r\nmalformed!\r\n')
+        self.assertRaises(ValueError, http.fromChunk, b'0xa\r\nmalformed!\r\n')
+        self.assertRaises(ValueError, http.fromChunk, b'0XA\r\nmalformed!\r\n')
 
     def testConcatenatedChunks(self):
         chunked = b''.join([b''.join(http.toChunk(t)) for t in self.strings])
@@ -1646,7 +1768,12 @@ class ParsingTests(unittest.TestCase):
         Line folded headers are handled by L{HTTPChannel} by replacing each
         fold with a single space by the time they are made available to the
         L{Request}. Any leading whitespace in the folded lines of the header
-        value is preserved.
+        value is replaced with a single space, per:
+
+            A server that receives an obs-fold in a request message ... MUST
+            ... replace each received obs-fold with one or more SP octets prior
+            to interpreting the field value or forwarding the message
+            downstream.
 
         See RFC 7230 section 3.2.4.
         """
@@ -1683,15 +1810,65 @@ class ParsingTests(unittest.TestCase):
         )
         self.assertEqual(
             request.requestHeaders.getRawHeaders(b"space"),
-            [b"space  space"],
+            [b"space space"],
         )
         self.assertEqual(
             request.requestHeaders.getRawHeaders(b"spaces"),
-            [b"spaces   spaces    spaces"],
+            [b"spaces spaces spaces"],
         )
         self.assertEqual(
             request.requestHeaders.getRawHeaders(b"tab"),
-            [b"t \ta \tb"],
+            [b"t a b"],
+        )
+
+    def test_headerStripWhitespace(self):
+        """
+        Leading and trailing space and tab characters are stripped from
+        headers. Other forms of whitespace are preserved.
+
+        See RFC 7230 section 3.2.3 and 3.2.4.
+        """
+        processed = []
+
+        class MyRequest(http.Request):
+            def process(self):
+                processed.append(self)
+                self.finish()
+
+        requestLines = [
+            b"GET / HTTP/1.0",
+            b"spaces:   spaces were stripped   ",
+            b"tabs: \t\ttabs were stripped\t\t",
+            b"spaces-and-tabs: \t \t spaces and tabs were stripped\t \t",
+            b"line-tab:   \v vertical tab was preserved\v\t",
+            b"form-feed: \f form feed was preserved \f  ",
+            b"",
+            b"",
+        ]
+
+        self.runRequest(b"\n".join(requestLines), MyRequest, 0)
+        [request] = processed
+        # All leading and trailing whitespace is stripped from the
+        # header-value.
+        self.assertEqual(
+            request.requestHeaders.getRawHeaders(b"spaces"),
+            [b"spaces were stripped"],
+        )
+        self.assertEqual(
+            request.requestHeaders.getRawHeaders(b"tabs"),
+            [b"tabs were stripped"],
+        )
+        self.assertEqual(
+            request.requestHeaders.getRawHeaders(b"spaces-and-tabs"),
+            [b"spaces and tabs were stripped"],
+        )
+        self.assertEqual(
+            request.requestHeaders.getRawHeaders(b"line-tab"),
+            [b"\v vertical tab was preserved\v"],
+        )
+        self.assertEqual(
+            request.requestHeaders.getRawHeaders(b"form-feed"),
+            [b"\f form feed was preserved \f"],
         )
 
 
@@ -2267,6 +2444,58 @@ Hello,
         ])
 
 
+    def test_contentLengthMalformed(self):
+        """
+        A request with a non-integer C{Content-Length} header fails with a 400
+        response without calling L{Request.process}.
+        """
+        self.assertRequestRejected(
+            [
+                b"GET /a HTTP/1.1",
+                b"Content-Length: MORE THAN NINE THOUSAND!",
+                b"Host: host.invalid",
+                b"",
+                b"",
+                b"x" * 9001,
+            ]
+        )
+
+    def test_contentLengthTooPositive(self):
+        """
+        A request with a C{Content-Length} header that begins with a L{+} fails
+        with a 400 response without calling L{Request.process}.
+
+        This is a potential request smuggling vector: see GHSA-c2jg-hw38-jrqq.
+        """
+        self.assertRequestRejected(
+            [
+                b"GET /a HTTP/1.1",
+                b"Content-Length: +100",
+                b"Host: host.invalid",
+                b"",
+                b"",
+                b"x" * 100,
+            ]
+        )
+
+    def test_contentLengthNegative(self):
+        """
+        A request with a C{Content-Length} header that is negative fails with
+        a 400 response without calling L{Request.process}.
+
+        This is a potential request smuggling vector: see GHSA-c2jg-hw38-jrqq.
+        """
+        self.assertRequestRejected(
+            [
+                b"GET /a HTTP/1.1",
+                b"Content-Length: -100",
+                b"Host: host.invalid",
+                b"",
+                b"",
+                b"x" * 200,
+            ]
+        )
+
     def test_duplicateContentLengthsWithPipelinedRequests(self):
         """
         Two pipelined requests, the first of which includes multiple
@@ -4226,3 +4455,43 @@ class HTTPClientSanitizationTests(unittest.SynchronousTestCase):
                 transport.value().splitlines(),
                 [b": ".join([sanitizedBytes, sanitizedBytes])]
             )
+
+
+class HexHelperTests(unittest.SynchronousTestCase):
+    """
+    Test the L{http._hexint} and L{http._ishexdigits} helper functions.
+    """
+
+    badStrings = (b"", b"0x1234", b"feds", b"-123" b"+123")
+
+    def test_isHex(self):
+        """
+        L{_ishexdigits()} returns L{True} for nonempy bytestrings containing
+        hexadecimal digits.
+        """
+        for s in (b"10", b"abcdef", b"AB1234", b"fed", b"123467890"):
+            self.assertIs(True, http._ishexdigits(s))
+
+    def test_decodes(self):
+        """
+        L{_hexint()} returns the integer equivalent of the input.
+        """
+        self.assertEqual(10, http._hexint(b"a"))
+        self.assertEqual(0x10, http._hexint(b"10"))
+        self.assertEqual(0xABCD123, http._hexint(b"abCD123"))
+
+    def test_isNotHex(self):
+        """
+        L{_ishexdigits()} returns L{False} for bytestrings that don't contain
+        hexadecimal digits, including the empty string.
+        """
+        for s in self.badStrings:
+            self.assertIs(False, http._ishexdigits(s))
+
+    def test_decodeNotHex(self):
+        """
+        L{_hexint()} raises L{ValueError} for bytestrings that can't
+        be decoded.
+        """
+        for s in self.badStrings:
+            self.assertRaises(ValueError, http._hexint, s)