You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mod_python-commits@quetz.apache.org by jg...@apache.org on 2006/03/05 18:32:03 UTC
svn commit: r383359 - in /httpd/mod_python/trunk: Doc/appendixc.tex
lib/python/mod_python/Session.py test/test.py
Author: jgallacher
Date: Sun Mar 5 09:32:01 2006
New Revision: 383359
URL: http://svn.apache.org/viewcvs?rev=383359&view=rev
Log:
Ported sid validity check fix for MODPYTHON-135 from branches/3.2.x to
trunk.
Modified:
httpd/mod_python/trunk/Doc/appendixc.tex
httpd/mod_python/trunk/lib/python/mod_python/Session.py
httpd/mod_python/trunk/test/test.py
Modified: httpd/mod_python/trunk/Doc/appendixc.tex
URL: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/Doc/appendixc.tex?rev=383359&r1=383358&r2=383359&view=diff
==============================================================================
--- httpd/mod_python/trunk/Doc/appendixc.tex (original)
+++ httpd/mod_python/trunk/Doc/appendixc.tex Sun Mar 5 09:32:01 2006
@@ -29,6 +29,9 @@
methods. These allows the dynamic registration of filters and the
attaching of filters to the current request.
\item
+ (\citetitle[http://issues.apache.org/jira/browse/MODPYTHON-135]{MODPYTHON-135})
+ Fixed possible directory traversal attack in FileSession.
+ \item
(\citetitle[http://issues.apache.org/jira/browse/MODPYTHON-137]{MODPYTHON-137})
New \code{req.server.get_options()} method. This returns the subset
of Python options set at global scope within the Apache configuration.
Modified: httpd/mod_python/trunk/lib/python/mod_python/Session.py
URL: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/lib/python/mod_python/Session.py?rev=383359&r1=383358&r2=383359&view=diff
==============================================================================
--- httpd/mod_python/trunk/lib/python/mod_python/Session.py (original)
+++ httpd/mod_python/trunk/lib/python/mod_python/Session.py Sun Mar 5 09:32:01 2006
@@ -29,6 +29,7 @@
import cPickle, cStringIO
import tempfile
import traceback
+import re
COOKIE_NAME="pysid"
DFT_TIMEOUT=30*60 # 30 min
@@ -82,6 +83,19 @@
# Make a number based on current time, pid, remote ip
# and two random ints, then hash with md5. This should
# be fairly unique and very difficult to guess.
+ #
+ # WARNING
+ # The current implementation of _new_sid returns an
+ # md5 hexdigest string. To avoid a possible directory traversal
+ # attack in FileSession the sid is validated using
+ # the _check_sid() method and the compiled regex
+ # validate_sid_re. The sid will be accepted only if len(sid) == 32
+ # and it only contains the characters 0-9 and a-f.
+ #
+ # If you change this implementation of _new_sid, make sure to also
+ # change the validation scheme, as well as the test_Session_illegal_sid()
+ # unit test in test/test.py.
+ # /WARNING
t = long(time.time()*10000)
pid = os.getpid()
@@ -92,6 +106,21 @@
return md5.new("%d%d%d%d%s" % (t, pid, rnd1, rnd2, ip)).hexdigest()
+validate_sid_re = re.compile('[0-9a-f]{32}$')
+
+def _check_sid(sid):
+ ## Check the validity of the session id
+ # # The sid must be 32 characters long, and consisting of the characters
+ # 0-9 and a-f.
+ #
+ # The sid may be passed in a cookie from the client and as such
+ # should not be trusted. This is particularly important in
+ # FileSession, where the session filename is derived from the sid.
+ # A sid containing '/' or '.' characters could result in a directory
+ # traversal attack
+
+ return not not validate_sid_re.match(sid)
+
class BaseSession(dict):
def __init__(self, req, sid=None, secret=None, lock=1,
@@ -120,6 +149,14 @@
if cookies.has_key(session_cookie_name):
self._sid = cookies[session_cookie_name].value
+
+ if self._sid:
+ # Validate the sid *before* locking the session
+ # _check_sid will raise an apache.SERVER_RETURN exception
+ if not _check_sid(self._sid):
+ self._req.log_error("mod_python.Session warning: The session id contains invalid characters, valid characters are only 0-9 and a-f",
+ apache.APLOG_WARNING)
+ raise apache.SERVER_RETURN, apache.HTTP_INTERNAL_SERVER_ERROR
self.init_lock()
Modified: httpd/mod_python/trunk/test/test.py
URL: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/test/test.py?rev=383359&r1=383358&r2=383359&view=diff
==============================================================================
--- httpd/mod_python/trunk/test/test.py (original)
+++ httpd/mod_python/trunk/test/test.py Sun Mar 5 09:32:01 2006
@@ -1841,10 +1841,47 @@
response = conn.getresponse()
rsp = response.read()
conn.close()
-
if rsp != "test ok":
self.fail("session did not accept our cookie")
+ def test_Session_illegal_sid_conf(self):
+
+ c = VirtualHost("*",
+ ServerName("test_Session_Session"),
+ DocumentRoot(DOCUMENT_ROOT),
+ Directory(DOCUMENT_ROOT,
+ SetHandler("mod_python"),
+ PythonHandler("tests::Session_Session"),
+ PythonDebug("On")))
+ return str(c)
+
+ def test_Session_illegal_sid(self):
+
+ print "\n * Testing Session with illegal session id value"
+ bad_cookie = 'pysid=/path/traversal/attack/bad; path=/'
+ conn = httplib.HTTPConnection("127.0.0.1:%s" % PORT)
+ conn.putrequest("GET", "/tests.py", skip_host=1)
+ conn.putheader("Host", "test_Session_Session:%s" % PORT)
+ conn.putheader("Cookie", bad_cookie)
+ conn.endheaders()
+ response = conn.getresponse()
+ status = response.status
+ conn.close()
+ if status != 500:
+ self.fail("session accepted a sid with illegal characters")
+
+ bad_cookie = 'pysid=%s; path=/' % ('abcdef'*64)
+ conn = httplib.HTTPConnection("127.0.0.1:%s" % PORT)
+ conn.putrequest("GET", "/tests.py", skip_host=1)
+ conn.putheader("Host", "test_Session_Session:%s" % PORT)
+ conn.putheader("Cookie", bad_cookie)
+ conn.endheaders()
+ response = conn.getresponse()
+ status = response.status
+ conn.close()
+ if status != 500:
+ self.fail("session accepted a sid which is too long")
+
def test_publisher_conf(self):
c = VirtualHost("*",
ServerName("test_publisher"),
@@ -2250,6 +2287,7 @@
perRequestSuite.addTest(PerRequestTestCase("test_Cookie_Cookie"))
perRequestSuite.addTest(PerRequestTestCase("test_Cookie_MarshalCookie"))
perRequestSuite.addTest(PerRequestTestCase("test_Session_Session"))
+ perRequestSuite.addTest(PerRequestTestCase("test_Session_illegal_sid"))
perRequestSuite.addTest(PerRequestTestCase("test_interpreter_per_directive"))
perRequestSuite.addTest(PerRequestTestCase("test_interpreter_per_directory"))
perRequestSuite.addTest(PerRequestTestCase("test_publisher"))