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

rfile.read(0) creates infinite loop + memory hog that crashes server if not killed fast enough #1602

Closed
mewalig opened this Issue Jun 6, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@mewalig

mewalig commented Jun 6, 2017

I'm submitting a bug report

When processing a POST with chunked encoding, read(0) on rfile will go into an infinite loop that hogs memory until the process is killed or the server crashes (I'm not sure whether this issue also exists with non-chunked encoding). I am using Django and sample code to reproduce is:

    if request.method == 'POST':
        f = request.META.get('wsgi.input')
        # f will be type cherrypy.wsgiserver.wsgiserver2.ChunkedRFile
       x = f.read(0)
       ...

sample curl to reproduce:

curl -X POST -d @bunch_of_data.dat 'http://...'

The problem seems to be in cheroot/server.py, where read() does not differentiate between size being None and size being zero. it goes into an infinite loop and the following line will eat up more and more memory until none is left:

data += self.buffer

To fix, insert the indicated lines below:

def read(self, size=None):
    data = EMPTY

    if size == 0:   ### INSERT THIS LINE
        return data ### INSERT THIS LINE

    while True:
        if size and len(data) >= size:
            return data

        if not self.buffer:
            self._fetch()
            if not self.buffer:
                # EOF
                return data

        if size:
            remaining = size - len(data)
            data += self.buffer[:remaining]
            self.buffer = self.buffer[remaining:]
        else:
            data += self.buffer

am running with Python version: 2.7 on osx

@webknjaz

This comment has been minimized.

Member

webknjaz commented Jun 6, 2017

Ref: #1486

@webknjaz webknjaz added the bug label Jun 6, 2017

@webknjaz

This comment has been minimized.

Member

webknjaz commented Jun 6, 2017

@mewalig could you please create a PR with a change you propose?

@mewalig

This comment has been minimized.

mewalig commented Jun 12, 2017

done
cherrypy/cheroot#30

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jul 19, 2017

wiz
Updated py-cheroot to 5.7.0.
v5.7.0
======

- CI improvements:
  * Don't run tests during deploy stage
  * Use VM based build job env only for pyenv envs
  * Opt-in for beta trusty image @ Travis CI
  * Be verbose when running tests (show test names)
  * Show xfail/skip details during test run

- #34: Fix ``_handle_no_ssl`` error handler calls

- #21: Fix ``test_conn`` tests:
  * Improve setup_server def in HTTP connection tests
  * Fix HTTP streaming tests
  * Fix HTTP/1.1 pipelining test under Python 3
  * Fix ``test_readall_or_close`` test
  * Fix ``test_No_Message_Body``
  * Clarify ``test_598`` fail reason

- #36: Add GitHub templates for PR, issue && contributing

- #27: Default HTTP Server header to Cheroot version str

- Cleanup _compat functions from server module

v5.6.0
======

- Fix all PEP 257 related errors in all non-test modules.

  ``cheroot/test/*`` folder is only one left allowed to fail with this linter.

- #30: Optimize chunked body reader loop by returning empty data is the size is 0.

  Ref: cherrypy/cherrypy#1602

- Reset buffer if the body size is unknown

  Ref: cherrypy/cherrypy#1486

- Add missing size hint to SizeCheckWrapper

  Ref: cherrypy/cherrypy#1131

v5.5.2
======

- #32: Ignore "unknown error" and "https proxy request" SSL errors.

  Ref: sabnzbd/sabnzbd#820

  Ref: sabnzbd/sabnzbd#860

v5.5.1
======

- Make Appveyor list separate tests in corresponding tab.

- #29: Configure Travis CI build stages.

  Prioritize tests by stages.

  Move deploy stage to be run very last after all other stages finish.

- #31: Ignore "Protocol wrong type for socket" (EPROTOTYPE) @ OSX for non-blocking sockets.

  This was originally fixed for regular sockets in cherrypy/cherrypy#1392.

  Ref: https://forums.sabnzbd.org/viewtopic.php?f=2&t=22728&p=112251

v5.5.0
======

- #17 via #25: Instead of a read_headers function, cheroot now
  supplies a HeaderReader class to perform the same function.

  Any HTTPRequest object may override the header_reader attribute
  to customize the handling of incoming headers.

  The server module also presents a provisional implementation of
  a DropUnderscoreHeaderReader that will exclude any headers
  containing an underscore. It remains an exercise for the
  implementer to demonstrate how this functionality might be
  employed in a server such as CherryPy.

- #26: Configured TravisCI to run tests under OS X.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment