Issue #874 new

[PATCH] Fix potential denial of service in session handling

Anonymous created an issue

CherryPy with 'tools.sessions.on = True' will currently create a new session for each request that arrives without a valid session cookie. This is undesirable because every session consumes server ressources (RAM, diskspace or slots in a LRU) and a malicious attack, or simply a high-traffic situation, can lead to denial of service for legitimate users.

For perspective: a single malicious client-host can enforce the creation of thousands of sessions per second using nothing but a simple request-generator like ab2 (apachebench).

The attached patch mitigates the problem by creating sessions only when absolutely necessary, that is when data has been written to the session-dict, e.g. via ''cherrypy.session['foo'] = bar''.

This gives full control over the session lifecycle to the application developer, who must still pay attention to not create sessions carelessly.

As a rule of thumb: Create sessions only after a successful login or captcha verification. Pages that are freely accessible for hammering should never create a session.

There is one backwards-incompatible change in this Changeset: Custom Session implementations (including subclasses of !RamSession, !MemcachedSession etc. that override !init!()) must add a check to the constructor that bails out if the session "disappears during initialization".

See an example:

{{{ class MyRamSession(RamSession) def init( self, id=None, kwargs ): RamSession.__init__( self, id, kwargs )

    ### this block is new
    if self.id is None:
       # this was only a test for existence, bail out
       return

    # do your thing here as usual...

}}}

Other than that the patch should be fully backwards compatible.

These are the new semantics for all possible cases:

{{{ Request : No session cookie Controller: No write to the session Result : No session is created, no cookie is sent

Request : No session cookie Controller: Writes to the session Result : Session is created, browser cookie sent

Request : Valid session cookie (session exists) Controller: Writes to the session Result : Session is maintained as usual

Request : Valid session cookie (session exists) Controller: No write to the session Result : Session is maintained as usual

Request : Invalid session cookie (invalid, expired or non-existing sessionid) Controller: No write to the session Result : No session is created, browser cookie is cleared

Request : Invalid session cookie (invalid, expired or non-existing sessionid) Controller: Writes to the session Result : Session is created, browser cookie is updated

}}}

TESTING: All unit tests pass and I have confirmed all cases manually with firefox/firecookie against my subclass of !MemcachedSession and a vanilla !RamSession.

Feedback and improvements welcome...

Reported by filter-cherrypy@mbox.bz

Comments (6)

  1. Robert Brewer

    Hm. You say "create sessions only after a successful login or captcha verification", but I can't tell whether your list of cases is authenticated or not or a mix of both. It looks like both. Can you double the number of cases, so that each one has both a verified auth case and an unverified case? That would help greatly.

  2. moe

    Sorry, authentication is completely irrelevant here. I should maybe have left that suggestion out of the original text to avoid confusion.

    The purpose of the patch is to have cherrypy only create a session when the app consciously decides to create one (by writing to the session dict) and not for ''every'' request (as it is now).

    That's really all there is to it. It doesn't matter at all ''why'' the app decides to create a session, that's up to the app developer. It only matters that cherrypy will not create a session when it doesn't need to.

  3. Log in to comment