Skip to content

gh-142533: Prevent CRLF injection in HTTP headers #142605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tadejmagajna wants to merge 5 commits into
base: main
Choose a base branch
from
+92 −25
Open

gh-142533: Prevent CRLF injection in HTTP headers #142605

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6d53fd6
gh-142533: Prevent CRLF injection in HTTP headers
tadejmagajna Dec 11, 2025
5e56689
gh-142533: Fix lint issues in news fragment
tadejmagajna Dec 11, 2025
6d661e8
Ensure CRLF check only performed on string headers
tadejmagajna Dec 11, 2025
2f0605a
gh-142533: Validate CRLF in send_response_only and add test
tadejmagajna Dec 12, 2025
1c0d5d7
gh-142533: Apply PR feedback about redundant code changes
tadejmagajna Dec 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
gh-142533: Prevent CRLF injection in HTTP headers 
Reject CR/LF in header names/values in `http.server` and `wsgiref.headers`
to prevent CRLF injection attacks.
  • Loading branch information
@tadejmagajna
tadejmagajna committed Dec 11, 2025
commit 6d53fd63f8d278a485f9801e64f88431f3802318
7 changes: 7 additions & 0 deletions Lib/http/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@

DEFAULT_ERROR_CONTENT_TYPE = "text/html;charset=utf-8"

def _validate_header_string(value):
"""Validate header values preventing CRLF injection."""
if '\r' in value or '\n' in value:
raise ValueError('Invalid header name/value: contains CR or LF')

class HTTPServer(socketserver.TCPServer):

allow_reuse_address = True # Seems to make sense in testing environment
Expand Down Expand Up @@ -553,6 +558,8 @@ def send_response_only(self, code, message=None):

def send_header(self, keyword, value):
"""Send a MIME header to the headers buffer."""
_validate_header_string(keyword)
_validate_header_string(value)
if self.request_version != 'HTTP/0.9':
if not hasattr(self, '_headers_buffer'):
self._headers_buffer = []
Expand Down
26 changes: 26 additions & 0 deletions Lib/test/test_httpservers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,32 @@ def test_header_buffering_of_send_header(self):
self.assertEqual(output.getData(), b'Foo: foo\r\nbar: bar\r\n\r\n')
self.assertEqual(output.numWrites, 1)

def test_crlf_injection_in_header_value(self):
input = BytesIO(b'GET / HTTP/1.1\r\n\r\n')
output = AuditableBytesIO()
handler = SocketlessRequestHandler()
handler.rfile = input
handler.wfile = output
handler.request_version = 'HTTP/1.1'

with self.assertRaises(ValueError) as ctx:
handler.send_header('X-Custom', 'value\r\nSet-Cookie: custom=true')
self.assertIn('Invalid header name/value: contains CR or LF',
str(ctx.exception))

def test_crlf_injection_in_header_name(self):
input = BytesIO(b'GET / HTTP/1.1\r\n\r\n')
output = AuditableBytesIO()
handler = SocketlessRequestHandler()
handler.rfile = input
handler.wfile = output
handler.request_version = 'HTTP/1.1'

with self.assertRaises(ValueError) as ctx:
handler.send_header('X-Inj\r\nSet-Cookie', 'value')
self.assertIn('Invalid header name/value: contains CR or LF',
str(ctx.exception))

def test_header_unbuffered_when_continue(self):

def _readAndReseek(f):
Expand Down
33 changes: 33 additions & 0 deletions Lib/test/test_wsgiref.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,39 @@ def testExtras(self):
'\r\n'
)

def test_crlf_rejection_in_setitem(self):
h = Headers()
for crlf in ('\r\n', '\r', '\n'):
with self.subTest(crlf_repr=repr(crlf)):
with self.assertRaises(ValueError) as ctx:
h['X-Custom'] = f'value{crlf}Set-Cookie: test'
self.assertIn('CR or LF', str(ctx.exception))

def test_crlf_rejection_in_setdefault(self):
for crlf in ('\r\n', '\r', '\n'):
with self.subTest(crlf_repr=repr(crlf)):
h = Headers()
with self.assertRaises(ValueError) as ctx:
h.setdefault('X-Custom', f'value{crlf}Set-Cookie: test')
self.assertIn('CR or LF', str(ctx.exception))

def test_crlf_rejection_in_add_header(self):
for crlf in ('\r\n', '\r', '\n'):
with self.subTest(location='value', crlf_repr=repr(crlf)):
h = Headers()
with self.assertRaises(ValueError) as ctx:
h.add_header('X-Custom', f'value{crlf}Set-Cookie: test')
self.assertIn('CR or LF', str(ctx.exception))

with self.subTest(location='param', crlf_repr=repr(crlf)):
h = Headers()
with self.assertRaises(ValueError) as ctx:
h.add_header('Content-Disposition',
'attachment',
filename=f'test{crlf}.txt')
self.assertIn('CR or LF', str(ctx.exception))


class ErrorHandler(BaseCGIHandler):
"""Simple handler subclass for testing BaseHandler"""

Expand Down
31 changes: 17 additions & 14 deletions Lib/wsgiref/headers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@ def __init__(self, headers=None):
self._headers = headers
if __debug__:
for k, v in headers:
self._convert_string_type(k)
self._convert_string_type(v)
self._validate_header_string(k)
self._validate_header_string(v)
Copy link
Member

@serhiy-storchaka serhiy-storchaka Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is debug-only check. It can be completely omitted.


def _convert_string_type(self, value):
"""Convert/check value type."""
def _validate_header_string(self, value):
"""Validate header type and value."""
if type(value) is str:
if '\r' in value or '\n' in value:
raise ValueError('Invalid header name/value: contains CR or LF')
return value
Copy link
Member

@serhiy-storchaka serhiy-storchaka Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you changed the method name, backward compatibility is not a concern. No need to return the value.

Copy link
Contributor Author

@tadejmagajna tadejmagajna Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This required a small refactor of the function code (i.e. no more early return) and a tiny change of all usages that relied on the return value

raise AssertionError("Header names/values must be"
" of type str (got {0})".format(repr(value)))
Expand All @@ -53,14 +55,15 @@ def __setitem__(self, name, val):
"""Set the value of a header."""
del self[name]
self._headers.append(
(self._convert_string_type(name), self._convert_string_type(val)))
(self._validate_header_string(name),
self._validate_header_string(val)))

def __delitem__(self,name):
"""Delete all occurrences of a header, if present.

Does *not* raise an exception if the header is missing.
"""
name = self._convert_string_type(name.lower())
name = self._validate_header_string(name.lower())
Copy link
Member

@serhiy-storchaka serhiy-storchaka Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially this code was added to handle errors of misusing 8-bit strings and unicode strings. But now we have a single string type, and bytes cannot slip through so easy -- they will cause other errors.

self._headers[:] = [kv for kv in self._headers if kv[0].lower() != name]

def __getitem__(self,name):
Expand All @@ -87,13 +90,13 @@ def get_all(self, name):
fields deleted and re-inserted are always appended to the header list.
If no fields exist with the given name, returns an empty list.
"""
name = self._convert_string_type(name.lower())
name = self._validate_header_string(name.lower())
return [kv[1] for kv in self._headers if kv[0].lower()==name]


def get(self,name,default=None):
"""Get the first header value for 'name', or return 'default'"""
name = self._convert_string_type(name.lower())
name = self._validate_header_string(name.lower())
for k,v in self._headers:
if k.lower()==name:
return v
Expand Down Expand Up @@ -148,8 +151,8 @@ def setdefault(self,name,value):
and value 'value'."""
result = self.get(name)
if result is None:
self._headers.append((self._convert_string_type(name),
self._convert_string_type(value)))
self._headers.append((self._validate_header_string(name),
self._validate_header_string(value)))
return value
else:
return result
Expand All @@ -172,13 +175,13 @@ def add_header(self, _name, _value, **_params):
"""
parts = []
if _value is not None:
_value = self._convert_string_type(_value)
_value = self._validate_header_string(_value)
parts.append(_value)
for k, v in _params.items():
k = self._convert_string_type(k)
k = self._validate_header_string(k)
if v is None:
parts.append(k.replace('_', '-'))
else:
v = self._convert_string_type(v)
v = self._validate_header_string(v)
parts.append(_formatparam(k.replace('_', '-'), v))
self._headers.append((self._convert_string_type(_name), "; ".join(parts)))
self._headers.append((self._validate_header_string(_name), "; ".join(parts)))
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2025-12-11-22-28-21.gh-issue-142533.SN0f9_.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Reject CR/LF in HTTP headers in `http.server` and `wsgiref.headers` to prevent
CRLF injection attacks.
Loading