Skip to content
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

assert only, no change intended: never reflect invalid input #3264

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

assert only, no change intended: never reflect invalid input #3264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e695948
assert: never reflect invalid input
pajod Aug 11, 2024
5194854
style: shut up pylint
pajod Nov 27, 2024
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
2 changes: 1 addition & 1 deletion .pylintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ disable=
broad-except,
duplicate-bases,
duplicate-code,
eval-used,
superfluous-parens,
fixme,
import-error,
import-outside-toplevel,
Expand Down
11 changes: 11 additions & 0 deletions gunicorn/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import urllib.parse

REDIRECT_TO = getattr(os, 'devnull', '/dev/null')
REASON_PHRASE_RE = re.compile(rb'[ \t\x21-\x7e\x80-\xff]*')

# Server and Date aren't technically hop-by-hop
# headers, but they are in the purview of the
Expand Down Expand Up @@ -307,6 +308,16 @@ def write_nonblock(sock, data, chunked=False):


def write_error(sock, status_int, reason, mesg):
# we may reflect user input in mesg
# .. as long as it is escaped appropriately for indicated Content-Type
# we should send our own reason text
# .. we shall never send misleading or invalid HTTP status lines
if not REASON_PHRASE_RE.fullmatch(reason.encode("latin-1")):
raise AssertionError("Attempted to return malformed error reason: %r" % (reason, ))
# we should avoid chosing status codes that are already in use
# indicating special handling in our proxies
if not (100 <= status_int <= 599): # RFC9110 15
raise AssertionError("Attempted to return invalid error status code: %r" % (status_int, ))
html_error = textwrap.dedent("""\
<html>
<head>
Expand Down
17 changes: 17 additions & 0 deletions tests/test_http.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,23 @@ def test_http_header_encoding():
with pytest.raises(UnicodeEncodeError):
mocked_socket.sendall(util.to_bytestring(header_str, "ascii"))

def test_http_reflected_xss_in_error():
""" If we put arbitrary user input into the HTTP status line, our proxy could get confused """

mocked_socket = mock.MagicMock()
with pytest.raises(UnicodeEncodeError):
util.write_error(
mocked_socket, 501,
"Not latin-1: \N{egg}",
"unused_",
)

with pytest.raises(AssertionError):
util.write_error(
mocked_socket, 501,
"Extra newline shall not appear in HTTP Status line: \n",
"harmless, will appear properly quoted in html",
)

def test_http_invalid_response_header():
""" tests whether http response headers are contains control chars """
Expand Down