classification
Title: HTTPServer does not correctly handle bad request line
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.4, Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: alex, christian.heimes, eric.araujo, ezio.melotti, maker, orsenthil, r.david.murray, terry.reedy
Priority: normal Keywords: patch

Created on 2012-09-28 10:54 by maker, last changed 2013-08-27 18:33 by maker.

Files
File name Uploaded Description Edit
issue16083.tests.patch maker, 2012-09-28 15:19 review
issue16083.patch maker, 2012-09-28 15:19 review
issue16083.1.patch maker, 2013-08-27 18:33
Messages (8)
msg171435 - (view) Author: Michele Orrù (maker) * Date: 2012-09-28 10:54
Sending a "GET /\0" causes a TypeEror to be raised and the connection to be unexpectedly closed. 

$ python -m SimpleHTTPServer 8000
$ printf "GET /\00" | nc localhost 8000

TypeError: must be encoded string without NULL bytes, not str
----------------------------------------

I think raising a 400 error should be fine. Also, shouldn't the error message contain a repr(string)?

[From http://corte.si/posts/code/pathod/pythonservers/index.html]
msg171472 - (view) Author: Michele Orrù (maker) * Date: 2012-09-28 14:15
Note: on python3, the error is 
  File "/[...]/cpython/Lib/genericpath.py", line 41, in isdir
    st = os.stat(s)
TypeError: embedded NUL character
(same exception but different message.)

I don't know where to start fixing, because the documentation for os.stat says "Perform the equivalent of a stat() system call on the given path.", which is not exactly the correct behavior in this case.

I see that 
$ printf "/\00" | xargs stat
stat()s correctly the root directory, and
$ printf "/\00tmp" | xargs stat
stat()s still '/'. So, is this a bug of os.stat?

Noising some coredevs.
msg171483 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-09-28 15:01
There has been some discussion about what the correct behavior of os.stat is, as well, I think.  Alex Gaynor raised a question about testing our behavior when nulls are present.

But clearly, if the desired behavior for url processing is different from the actual behavior of os.stat, you need to catch the error and turn it into the correct response.  I don't think we can change this aspect of the behavior of os.stat for a bug fix, even if we decide we want to.
msg171488 - (view) Author: Michele Orrù (maker) * Date: 2012-09-28 15:19
Attaching tests that asserts the issue, and a patch for http.server. 
Works on tip.
Should be ported also to 2.x?

Note: that 'f = None' is unnecessary, maybe an isolated commit for that?
msg172896 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-10-14 18:14
Is this really a security issue? If so, that should be explained.
msg172901 - (view) Author: Michele Orrù (maker) * Date: 2012-10-14 18:36
Well, it is a security issue IMO, but not particularly harmful. But certainly that's not a RFC violation, since I'm not sending rfc-compliant packets.[0]  

The best an attacker could do is to DDoS the server running HTTPServer: tracebacks may open file descriptors and/or send emails to the sysadmin, and hence the attacker could flood the server opening new file descriptors, or the email box.[0]
At least, this is the worst thing that came to my mind discussing with exarkun. 

[0] https://twistedmatrix.com/trac/ticket/6029
msg172913 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-10-14 20:18
This does not seem to qualify as a security issue.
msg196307 - (view) Author: Michele Orrù (maker) * Date: 2013-08-27 18:33
Still is an issue, though. Exported on the current tip.
History
Date User Action Args
2013-08-27 18:33:38makersetfiles: + issue16083.1.patch

messages: + msg196307
versions: + Python 3.5, - Python 2.7, Python 3.2, Python 3.3
2012-10-14 20:18:40eric.araujosettype: security -> behavior
title: HTTPServer does not correctly handle bad headers -> HTTPServer does not correctly handle bad request line
messages: + msg172913
versions: - Python 3.1
2012-10-14 18:36:50makersetmessages: + msg172901
2012-10-14 18:17:24pitrousetnosy: + orsenthil
2012-10-14 18:14:09terry.reedysetnosy: + terry.reedy
messages: + msg172896
2012-09-28 15:19:56makersetfiles: + issue16083.patch
2012-09-28 15:19:45makersetfiles: + issue16083.tests.patch
keywords: + patch
messages: + msg171488
2012-09-28 15:01:06r.david.murraysetnosy: + alex
messages: + msg171483
2012-09-28 14:15:03makersetnosy: + ezio.melotti, eric.araujo, r.david.murray
messages: + msg171472
2012-09-28 13:31:20makersetnosy: - exarkun
2012-09-28 13:29:37makersetnosy: + exarkun
2012-09-28 13:19:46christian.heimessetnosy: + christian.heimes
2012-09-28 10:54:02makercreate