From 1493c7db740b92fde383f5b75e0ed3ecf278d48b Mon Sep 17 00:00:00 2001 From: "U-ASPECTSECURITY\\mhoopes" Date: Thu, 6 Mar 2014 09:29:17 -0700 Subject: [PATCH] Added validation of session key format Added a check to ensure the session key contains only alphanumeric characters, '_', or '-'. This is to prevent a minor directory traversal issue where cache files could be forced to be created in the data_dir or the parent of file_dir. Included a simple test to demonstrate the issue/assert it is fixed. --- beaker/session.py | 4 ++++ tests/test_cookie_traversal.py | 42 ++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 tests/test_cookie_traversal.py diff --git a/beaker/session.py b/beaker/session.py index e14a6bb1..e9b9b4b0 100644 --- a/beaker/session.py +++ b/beaker/session.py @@ -2,6 +2,7 @@ import os from datetime import datetime, timedelta import time +from re import match from beaker.crypto import hmac as HMAC, hmac_sha1 as SHA1, sha1 from beaker import crypto, util from beaker.cache import clsmap @@ -153,6 +154,9 @@ def __init__(self, request, id=None, invalidate_corrupt=False, if not self.id and self.key in self.cookie: self.id = self.cookie[self.key].value + if self.id and not match("^[0-9a-zA-Z-_]+\Z", self.id): + self.id = None + self.is_new = self.id is None if self.is_new: self._create_id() diff --git a/tests/test_cookie_traversal.py b/tests/test_cookie_traversal.py new file mode 100644 index 00000000..565c2099 --- /dev/null +++ b/tests/test_cookie_traversal.py @@ -0,0 +1,42 @@ +# -*- coding: utf-8 -*- +import sys +import time +import warnings +import os + +from beaker.session import Session + + +def get_session(**kwargs): + """A shortcut for creating :class:`Session` instance""" + options = {} + options.update(**kwargs) + return Session({}, **options) + +def test_file_traversal_cookie(): + session = get_session(id='..traversed', type='file', data_dir='.') + session[u'traversal'] = u'True' + session.save() + assert not os.path.exists('./..traversed.cache') + + +def test_save_load(): + """Test if the data is actually persistent across requests""" + session = get_session(type='file', data_dir='.') + session[u'Suomi'] = u'Kimi Räikkönen' + session[u'Great Britain'] = u'Jenson Button' + session[u'Deutchland'] = u'Sebastian Vettel' + session.save() + + session = get_session(id=session.id, type='file', data_dir='.') + assert u'Suomi' in session + assert u'Great Britain' in session + assert u'Deutchland' in session + + assert session[u'Suomi'] == u'Kimi Räikkönen' + assert session[u'Great Britain'] == u'Jenson Button' + assert session[u'Deutchland'] == u'Sebastian Vettel' + +test_save_load() +test_file_traversal_cookie() +