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.

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

Waitress does not reject messages with BWS after field-name #273

Closed
rogueresistor opened this issue Dec 4, 2019 · 5 comments
Closed

Waitress does not reject messages with BWS after field-name #273

rogueresistor opened this issue Dec 4, 2019 · 5 comments

Comments

@rogueresistor
Copy link

@rogueresistor rogueresistor commented Dec 4, 2019

RFC 7230: server MUST reject messages with BWS after field-name (#445)
    Obey the RFC requirement to reject HTTP requests with whitespace
    between field-name and the colon delimiter. Rejection is
    critical in the presence of broken HTTP agents that mishandle
    malformed messages.

This is not currently handled in:

waitress/waitress/parser.py

Lines 163 to 197 in d20f4db

def parse_header(self, header_plus):
"""
Parses the header_plus block of text (the headers plus the
first line of the request).
"""
index = header_plus.find(b'\n')
if index >= 0:
first_line = header_plus[:index].rstrip()
header = header_plus[index + 1:]
else:
first_line = header_plus.rstrip()
header = b''
self.first_line = first_line # for testing
lines = get_header_lines(header)
headers = self.headers
for line in lines:
index = line.find(b':')
if index > 0:
key = line[:index]
if b'_' in key:
continue
value = line[index + 1:].strip()
key1 = tostr(key.upper().replace(b'-', b'_'))
# If a header already exists, we append subsequent values
# seperated by a comma. Applications already need to handle
# the comma seperated values, as HTTP front ends might do
# the concatenation for you (behavior specified in RFC2616).
try:
headers[key1] += tostr(b', ' + value)
except KeyError:
headers[key1] = tostr(value)
# else there's garbage in the headers?

This should be updated to properly reject the HTTP request.

@bertjwregeer

This comment has been minimized.

Copy link
Member

@bertjwregeer bertjwregeer commented Dec 13, 2019

Due to not stripping the key we end up with a dictionary key that has a space in it. So thankfully this can't cause any confusion.

@bertjwregeer

This comment has been minimized.

Copy link
Member

@bertjwregeer bertjwregeer commented Dec 13, 2019

Foo : bar
Foo: bar

Are treated as two different headers.

@mmerickel

This comment has been minimized.

Copy link
Member

@mmerickel mmerickel commented Dec 19, 2019

@bertjwregeer fixed in 1.4 I think?

@bertjwregeer

This comment has been minimized.

Copy link
Member

@bertjwregeer bertjwregeer commented Dec 19, 2019

@bertjwregeer

This comment has been minimized.

Copy link
Member

@bertjwregeer bertjwregeer commented Dec 20, 2019

This has been fixed with the new 1.4.0 release that was just pushed to PyPi.

This was referenced Dec 25, 2019
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this issue Jan 1, 2020
https://build.opensuse.org/request/show/758618
by user dimstar_suse
- update to 1.4.0:
  - Waitress used to slam the door shut on HTTP pipelined requests without
  setting the ``Connection: close`` header as appropriate in the response. This
  is of course not very friendly. Waitress now explicitly sets the header when
  responding with an internally generated error such as 400 Bad Request or 500
  Internal Server Error to notify the remote client that it will be closing the
  connection after the response is sent.
  - Waitress no longer allows any spaces to exist between the header field-name
  and the colon. While waitress did not strip the space and thereby was not
  vulnerable to any potential header field-name confusion, it should have sent
  back a 400 Bad Request. See Pylons/waitress#273
  - CRLR handling Security fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.