[issue11259] asynchat does not check if terminator is negative integer
Roundup Robot added the comment: New changeset f67df13dd512 by Victor Stinner in branch '3.4': Issue #11259: asynchat.async_chat().set_terminator() now raises a ValueError if http://hg.python.org/cpython/rev/f67df13dd512 New changeset d164fda9063a by Victor Stinner in branch 'default': (Merge 3.4) Issue #11259: asynchat.async_chat().set_terminator() now raises a http://hg.python.org/cpython/rev/d164fda9063a -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
STINNER Victor added the comment: This issue is now fixed, thanks for the report. Sorry for the delay :-( Asy Mark wrote, asynchat is now deprecated: it's time to switch to the new shiny asyncio module! -- nosy: +haypo resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Mark Lawrence added the comment: I've no objection to people trying to take this forward but they should be aware that asyncio is recommended for new code. -- nosy: +BreamoreBoy versions: +Python 3.4, Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Changes by Devin Cook devin.c.c...@gmail.com: Removed file: http://bugs.python.org/file29202/asynchat_tip.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Devin Cook added the comment: updating the patch to the current tip -- Added file: http://bugs.python.org/file34310/asynchat_tip.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Devin Cook added the comment:
I agree that this is probably a bug, but can't think of any instances where
this in itself would cause a security issue. By sending something like a
negative Content-Length, you do indeed get data returned that doesn't really
match the data sent on the wire. If you're able to manipulate the
Content-Length, though, instead of sending a negative value num, you could
instead send len(data) + num.
Here's a simple example I was able to come up with:
Server reads data and runs echo -n {data} (or any write the file specified
in data).
Client is supposed to send Content-Length, then that many bytes, expected to be
a file that should be written to.
Client instead sends -4\n/etc/passwd.bak.
Server runs echo -n /etc/passwd.
So that's certainly unexpected bahavior. However, this is a fairly low-level
module, and doesn't actually do anything with the data it collects. That's left
to the subclass, and subclasses should be responsible for validating any data
read off the wire before using it.
Attached is a patch to tip, including a new test case.
--
nosy: +devin
type: security - behavior
Added file: http://bugs.python.org/file29202/asynchat_tip.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue11259
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Марк Коренберг socketp...@gmail.com added the comment: I've never used negative terminators in asynchat and I'm not even sure why one would want to. no one wants :), but terminator numeric value may be achieved from the net, and hackers sometimes use such technique. attached shorttest.py do the test. If negative terminator is passed, ValueError exception will raise on patched version, and will not raise in unpatched. Currently, I studying test system for python, but I think you will add such simple test faster and cleaner. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Giampaolo Rodola' g.rod...@gmail.com added the comment: Can you write an actual patch which includes tests? Also, I think the z.patch in attachment is targeted for python 2.x as it does not apply cleanly against the current trunk. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Changes by Марк Коренберг socketp...@gmail.com: Added file: http://bugs.python.org/file20987/z31.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Марк Коренберг socketp...@gmail.com added the comment: actual patch which includes tests I do not understand you. Probably I can not write that patch. Do not know how to. Sorry :( -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Giampaolo Rodola' g.rod...@gmail.com added the comment: By writing a test I mean adding a unittest-based test case in Lib/test/test_asynchat.py which fails before fixing Lib/asynchat.py and succeeds afterwards. Now, I'm not even sure I properly understood your bug exactly. I've never used negative terminators in asynchat and I'm not even sure why one would want to. In this case, taking a look at a test would help me (and others) to understand what you are complaining about exactly and figure out whether this is actually a bug and if it is worth fixing it. As for how to properly write a patch see: http://www.python.org/dev/faq/ All new patches should be applied to python 3.3 first so every time you submit a new one you should work on the 3.3 branch (current trunk) which is: http://svn.python.org/projects/python/branches/py3k/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Марк Коренберг socketp...@gmail.com added the comment: Real patch is the first hunk of attached file. Other 2 hunks are optimizations.. -- keywords: +patch Added file: http://bugs.python.org/file20912/z.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Changes by Марк Коренберг socketp...@gmail.com: Removed file: http://bugs.python.org/file20912/z.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Марк Коренберг socketp...@gmail.com added the comment: only first hunk is really the patch. 2 next hunks are optimizations. -- Added file: http://bugs.python.org/file20915/z.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Марк Коренберг socketp...@gmail.com added the comment: = ORIGINAL === $ ./qwe.py 10 read length: 10 read data: xx should read test. read: test $ ./qwe.py -10 read length: -10 read data: xx should read test. read: test = PATCHED === $ ./qwe.py 10 read length: 10 read data: xx should read test. read: test $ ./qwe.py -10 read length: -10 error: uncaptured python exception, closing channel __main__.http_request_handler connected '' at 0x7fe69b9bf878 (type 'exceptions.ValueError':Negative terminator value is not allowed [/usr/lib/python2.6/asyncore.py|read|78] [/usr/lib/python2.6/asyncore.py|handle_read_event|428] [/tmp/qwe/asynchat.py|handle_read|160] [./qwe.py|found_terminator|19] [/tmp/qwe/asynchat.py|set_terminator|98]) root@fad:/tmp/qwe# -- Added file: http://bugs.python.org/file20916/qwe.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Changes by Марк Коренберг socketp...@gmail.com: Added file: http://bugs.python.org/file20917/shorttest.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Giampaolo Rodola' g.rod...@gmail.com added the comment: Can you provide a patch including a test case? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Changes by Марк Коренберг socketp...@gmail.com: -- title: asynchat - asynchat does not check if terminator is negative integer ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Changes by Antoine Pitrou pit...@free.fr: -- assignee: - giampaolo.rodola nosy: +giampaolo.rodola ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Giampaolo Rodola' g.rod...@gmail.com added the comment: What do you mean by constructions like self.ac_in_buffer[:n] will lead to misbehaviour.? Please try to be more precise (e.g. by providing a piece of code which demonstrates the issue). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11259] asynchat does not check if terminator is negative integer
Марк Коренберг socketp...@gmail.com added the comment: asynchat.py: class async_chat: handle_read(): --- elif isinstance(terminator, int) or isinstance(terminator, long): # numeric terminator n = terminator if lb n: self.collect_incoming_data (self.ac_in_buffer) self.ac_in_buffer = '' self.terminator = self.terminator - lb else: self.collect_incoming_data (self.ac_in_buffer[:n]) self.ac_in_buffer = self.ac_in_buffer[n:] self.terminator = 0 self.found_terminator() -- suppose, terminator is -10. if lb n never match. So, else branch executed. next, it will call self.collect_incoming_data (self.ac_in_buffer[:n]), to push data to user. It should push some data from beginning of the buffer, intead of this, total buffer except last 10 characters pushed. Moreover, self.ac_in_buffer = self.ac_in_buffer[n:] shoudl give tail of the buffer, ut instead of this, self.ac_in_buffer will contain part of the tail. Such behaviour may break protocol parsing. In my case, malicious user pass 'Content-Length: -100' and totally break protocol parsing. Crafted values may gain memory leak. In any way, author of this code does not thought about negative n in constructions [:n] or [n:]. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11259 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
