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 nl...@apache.org on 2005/01/29 12:07:02 UTC
svn commit: r149024 - /httpd/mod_python/trunk/lib/python/mod_python/publisher.py /httpd/mod_python/trunk/lib/python/mod_python/util.py /httpd/mod_python/trunk/src/include/mpversion.h /httpd/mod_python/trunk/test/htdocs/tests.py /httpd/mod_python/trunk/test/test.py
Author: nlehuen
Date: Sat Jan 29 03:07:00 2005
New Revision: 149024
URL: http://svn.apache.org/viewcvs?view=rev&rev=149024
Log:
- Rewriting mod_python/publisher.py resolve_object to enhance security.
- Added unit tests for mod_python/publisher.py
- Fixing [#MODPYTHON-13]
- Preparing 3.1.4 release
Modified:
httpd/mod_python/trunk/lib/python/mod_python/publisher.py
httpd/mod_python/trunk/lib/python/mod_python/util.py
httpd/mod_python/trunk/src/include/mpversion.h
httpd/mod_python/trunk/test/htdocs/tests.py
httpd/mod_python/trunk/test/test.py
Modified: httpd/mod_python/trunk/lib/python/mod_python/publisher.py
Url: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/lib/python/mod_python/publisher.py?view=diff&rev=149024&p1=httpd/mod_python/trunk/lib/python/mod_python/publisher.py&r1=149023&p2=httpd/mod_python/trunk/lib/python/mod_python/publisher.py&r2=149024
==============================================================================
--- httpd/mod_python/trunk/lib/python/mod_python/publisher.py (original)
+++ httpd/mod_python/trunk/lib/python/mod_python/publisher.py Sat Jan 29 03:07:00 2005
@@ -60,8 +60,14 @@
func_path = "index"
# if any part of the path begins with "_", abort
+ # We need to make this test here, before resolve_object,
+ # to prevent the loading of modules whose name begins with
+ # an underscore.
if func_path[0] == '_' or func_path.count("._"):
- raise apache.SERVER_RETURN, apache.HTTP_NOT_FOUND
+ req.log_error('Cannot access %s because '
+ 'it contains at least an underscore'
+ %func_path,apache.APLOG_WARNING)
+ raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
## import the script
path, module_name = os.path.split(req.filename)
@@ -259,19 +265,46 @@
This function traverses the objects separated by .
(period) to find the last one we're looking for.
"""
+ parts = object_str.split('.')
+ last_part = len(parts)-1
+
+ for i, obj_str in enumerate(parts):
+ # path components starting with an underscore are forbidden
+ if obj_str[0]=='_':
+ req.log_error('Cannot traverse %s in %s because '
+ 'it starts with an underscore'
+ %(obj_str,req.unparsed_uri),apache.APLOG_WARNING)
+ raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
- for obj_str in object_str.split('.'):
+ # if we're not in the first object (which is the module)
+ if i>0:
+ # we must be in an instance, nothing else
+ # we have to check for old-style instances AND
+ # new-style instances.
+ # XXX testing for new-style class instance is tricky
+ # see http://groups.google.fr/groups?th=7bab336f2b4f7e03&seekm=107l13c5tti8876%40news.supernews.com
+ if not (isinstance(obj,InstanceType)):
+ req.log_error('Cannot traverse %s in %s because '
+ '%s is not an instance'
+ %(obj_str,req.unparsed_uri,obj),apache.APLOG_WARNING)
+ raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
+
+ # we know it's OK to call getattr
+ # note that getattr can really call some code because
+ # of property objects (or attribute with __get__ special methods)...
obj = getattr(obj, obj_str)
- # object cannot be a module
- if type(obj) == ModuleType:
- raise apache.SERVER_RETURN, apache.HTTP_NOT_FOUND
+ # we don't want to get a module or class
+ obj_type = type(obj)
+ if (isinstance(obj,ModuleType)
+ or isinstance(obj,ClassType)
+ or isinstance(obj,type)):
+ req.log_error('Cannot access %s in %s because '
+ 'it is a class or module'
+ %(obj_str,req.unparsed_uri),apache.APLOG_WARNING)
+ raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
realm, user, passwd = process_auth(req, obj, realm,
user, passwd)
return obj
-
-
-
-
Modified: httpd/mod_python/trunk/lib/python/mod_python/util.py
Url: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/lib/python/mod_python/util.py?view=diff&rev=149024&p1=httpd/mod_python/trunk/lib/python/mod_python/util.py&r1=149023&p2=httpd/mod_python/trunk/lib/python/mod_python/util.py&r2=149024
==============================================================================
--- httpd/mod_python/trunk/lib/python/mod_python/util.py (original)
+++ httpd/mod_python/trunk/lib/python/mod_python/util.py Sat Jan 29 03:07:00 2005
@@ -336,10 +336,11 @@
# and for that we need to get a list of them. There
# are a few options for callable objects here:
- if type(object) is InstanceType:
+ if isinstance(object,InstanceType):
# instances are callable when they have __call__()
object = object.__call__
+ fc = None
expected = []
if hasattr(object, "func_code"):
# function
@@ -353,13 +354,15 @@
# class
fc = object.__init__.im_func.func_code
expected = fc.co_varnames[1:fc.co_argcount]
-
+
# remove unexpected args unless co_flags & 0x08,
# meaning function accepts **kw syntax
- if not (fc.co_flags & 0x08):
+ if not fc:
+ args = {}
+ elif not (fc.co_flags & 0x08):
for name in args.keys():
if name not in expected:
- del args[name]
+ del args[name]
return object(**args)
Modified: httpd/mod_python/trunk/src/include/mpversion.h
Url: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/src/include/mpversion.h?view=diff&rev=149024&p1=httpd/mod_python/trunk/src/include/mpversion.h&r1=149023&p2=httpd/mod_python/trunk/src/include/mpversion.h&r2=149024
==============================================================================
--- httpd/mod_python/trunk/src/include/mpversion.h (original)
+++ httpd/mod_python/trunk/src/include/mpversion.h Sat Jan 29 03:07:00 2005
@@ -1,5 +1,5 @@
#define MPV_MAJOR 3
#define MPV_MINOR 1
-#define MPV_PATCH 3
+#define MPV_PATCH 4
#define MPV_BUILD 0
-#define MPV_STRING "3.1.3"
+#define MPV_STRING "3.1.4"
Modified: httpd/mod_python/trunk/test/htdocs/tests.py
Url: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/test/htdocs/tests.py?view=diff&rev=149024&p1=httpd/mod_python/trunk/test/htdocs/tests.py&r1=149023&p2=httpd/mod_python/trunk/test/htdocs/tests.py&r2=149024
==============================================================================
--- httpd/mod_python/trunk/test/htdocs/tests.py (original)
+++ httpd/mod_python/trunk/test/htdocs/tests.py Sat Jan 29 03:07:00 2005
@@ -64,6 +64,10 @@
import os
import cStringIO
+# This is used for mod_python.publisher security tests
+_SECRET_PASSWORD = 'root'
+__ANSWER = 42
+
class SimpleTestCase(unittest.TestCase):
def __init__(self, methodName, req):
@@ -823,6 +827,26 @@
def interpreter(req):
req.write(req.interpreter)
return apache.OK
+
+def index(req):
+ return "test ok, interpreter=%s"%req.interpreter
+
+def test_publisher(req):
+ return "test ok, interpreter=%s"%req.interpreter
+
+class OldStyleClassTest:
+ def __call__(self,req):
+ return "test callable old-style instance ok"
+ def traverse(self,req):
+ return "test traversable old-style instance ok"
+old_instance = OldStyleClassTest()
+
+class InstanceTest(object):
+ def __call__(self,req):
+ return "test callable instance ok"
+ def traverse(self,req):
+ return "test traversable instance ok"
+instance = InstanceTest()
def _test_table():
Modified: httpd/mod_python/trunk/test/test.py
Url: http://svn.apache.org/viewcvs/httpd/mod_python/trunk/test/test.py?view=diff&rev=149024&p1=httpd/mod_python/trunk/test/test.py&r1=149023&p2=httpd/mod_python/trunk/test/test.py&r2=149024
==============================================================================
--- httpd/mod_python/trunk/test/test.py (original)
+++ httpd/mod_python/trunk/test/test.py Sat Jan 29 03:07:00 2005
@@ -1103,6 +1103,119 @@
if rsp != "test ok":
self.fail("session did not accept our cookie")
+ def test_publisher_conf(self):
+ c = VirtualHost("*",
+ ServerName("test_publisher"),
+ DocumentRoot(DOCUMENT_ROOT),
+ Directory(DOCUMENT_ROOT,
+ SetHandler("mod_python"),
+ PythonHandler("mod_python.publisher"),
+ PythonDebug("On")))
+ return str(c)
+
+ def test_publisher(self):
+ print "\n * Testing mod_python.publisher"
+
+ rsp = self.vhost_get("test_publisher", path="/tests.py")
+ if (rsp != "test ok, interpreter=test_publisher"):
+ self.fail(`rsp`)
+
+ rsp = self.vhost_get("test_publisher", path="/tests.py/index")
+ if (rsp != "test ok, interpreter=test_publisher"):
+ self.fail(`rsp`)
+
+ rsp = self.vhost_get("test_publisher", path="/tests.py/test_publisher")
+ if (rsp != "test ok, interpreter=test_publisher"):
+ self.fail(`rsp`)
+
+ def test_publisher_security_conf(self):
+ c = VirtualHost("*",
+ ServerName("test_publisher"),
+ DocumentRoot(DOCUMENT_ROOT),
+ Directory(DOCUMENT_ROOT,
+ SetHandler("mod_python"),
+ PythonHandler("mod_python.publisher"),
+ PythonDebug("On")))
+ return str(c)
+
+ def test_publisher_security(self):
+ print "\n * Testing mod_python.publisher security"
+
+ def get_status(path):
+ conn = httplib.HTTPConnection("127.0.0.1:%s" % PORT)
+ conn.putrequest("GET", path, skip_host=1)
+ conn.putheader("Host", "test_publisher:%s" % PORT)
+ conn.endheaders()
+ response = conn.getresponse()
+ status, response = response.status, response.read()
+ conn.close()
+ return status, response
+
+ status, response = get_status("/tests.py/_SECRET_PASSWORD")
+ if status != 403:
+ self.fail('Vulnerability : underscore prefixed attribute (%i)\n%s'%(status,response))
+
+ status, response = get_status("/tests.py/__ANSWER")
+ if status != 403:
+ self.fail('Vulnerability : underscore prefixed attribute (%i)\n%s'%(status,response))
+
+ status, response = get_status("/tests.py/re")
+ if status != 403:
+ self.fail('Vulnerability : module access (%i)\n%s'%(status,response))
+
+ status, response = get_status("/tests.py/OldStyleClassTest")
+ if status != 403:
+ self.fail('Vulnerability : old style class access (%i)\n%s'%(status,response))
+
+ status, response = get_status("/tests.py/InstanceTest")
+ if status != 403:
+ self.fail('Vulnerability : new style class access (%i)\n%s'%(status,response))
+
+ status, response = get_status("/tests.py/index/func_code")
+ if status != 403:
+ self.fail('Vulnerability : function traversal (%i)\n%s'%(status,response))
+
+ def test_publisher_old_style_instance_conf(self):
+ c = VirtualHost("*",
+ ServerName("test_publisher"),
+ DocumentRoot(DOCUMENT_ROOT),
+ Directory(DOCUMENT_ROOT,
+ SetHandler("mod_python"),
+ PythonHandler("mod_python.publisher"),
+ PythonDebug("On")))
+ return str(c)
+
+ def test_publisher_old_style_instance(self):
+ print "\n * Testing mod_python.publisher old-style instance publishing"
+
+ rsp = self.vhost_get("test_publisher", path="/tests.py/old_instance")
+ if (rsp != "test callable old-style instance ok"):
+ self.fail(`rsp`)
+
+ rsp = self.vhost_get("test_publisher", path="/tests.py/old_instance/traverse")
+ if (rsp != "test traversable old-style instance ok"):
+ self.fail(`rsp`)
+
+ def test_publisher_instance_conf(self):
+ c = VirtualHost("*",
+ ServerName("test_publisher"),
+ DocumentRoot(DOCUMENT_ROOT),
+ Directory(DOCUMENT_ROOT,
+ SetHandler("mod_python"),
+ PythonHandler("mod_python.publisher"),
+ PythonDebug("On")))
+ return str(c)
+
+ def test_publisher_instance(self):
+ print "\n * Testing mod_python.publisher instance publishing"
+
+ rsp = self.vhost_get("test_publisher", path="/tests.py/instance")
+ if (rsp != "test callable instance ok"):
+ self.fail(`rsp`)
+
+ rsp = self.vhost_get("test_publisher", path="/tests.py/instance/traverse")
+ if (rsp != "test traversable instance ok"):
+ self.fail(`rsp`)
class PerInstanceTestCase(unittest.TestCase, HttpdCtrl):
# this is a test case which requires a complete
@@ -1197,6 +1310,10 @@
perRequestSuite.addTest(PerRequestTestCase("test_Session_Session"))
perRequestSuite.addTest(PerRequestTestCase("test_interpreter_per_directive"))
perRequestSuite.addTest(PerRequestTestCase("test_interpreter_per_directory"))
+ perRequestSuite.addTest(PerRequestTestCase("test_publisher"))
+ perRequestSuite.addTest(PerRequestTestCase("test_publisher_security"))
+ perRequestSuite.addTest(PerRequestTestCase("test_publisher_old_style_instance"))
+ perRequestSuite.addTest(PerRequestTestCase("test_publisher_instance"))
# this must be last so its error_log is not overwritten
perRequestSuite.addTest(PerRequestTestCase("test_internal"))
Re: whitespace
Posted by "Michael C. Neel" <ne...@mediapulse.com>.
On Sat, 2005-01-29 at 13:01, Gregory (Grisha) Trubetskoy wrote:
> A bit of nitpicking :)
Pick the nits =p
http://python.org/peps/pep-0008.html
You're in good company ;)
Mike
whitespace
Posted by "Gregory (Grisha) Trubetskoy" <gr...@apache.org>.
A bit of nitpicking :)
I like to see more blank lines, comments and whitespace. Especially spaces
after commas, around '%' or in comparison. I don't have any specific
strict rules personally, I just like to take time to make sure code looks
nice and readable. I've been also neglecting length of line restrictions
in mod_python, some lines are quite long because I think it looks clearer.
E.g. instead of:
if i>0:
# we must be in an instance, nothing else
# we have to check for old-style instances AND
# new-style instances.
# XXX testing for new-style class instance is tricky
# see http://groups.google.fr/groups?th=7bab336f2b4f7e03&seekm=107l13c5tti8876%40news.supernews.com
if not (isinstance(obj,InstanceType)):
req.log_error('Cannot traverse %s in %s because '
'%s is not an instance'
%(obj_str,req.unparsed_uri,obj),apache.APLOG_WARNING)
raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
something like:
if i > 0:
# we must be in an instance, nothing else
# we have to check for old-style instances AND new-style instances.
# XXX testing for new-style class instance is tricky
# see http://groups.google.fr/groups?th=7bab336f2b4f7e03&seekm=107l13c5tti8876%40news.supernews.com
if not (isinstance(obj, InstanceType)):
req.log_error('Cannot traverse %s in %s because %s is not an instance' \
% (obj_str, req.unparsed_uri, obj), apache.APLOG_WARNING)
raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
On Sat, 29 Jan 2005 nlehuen@apache.org wrote:
> + for i, obj_str in enumerate(parts):
> + # path components starting with an underscore are forbidden
> + if obj_str[0]=='_':
> + req.log_error('Cannot traverse %s in %s because '
> + 'it starts with an underscore'
> + %(obj_str,req.unparsed_uri),apache.APLOG_WARNING)
> + raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
>
> - for obj_str in object_str.split('.'):
> + # if we're not in the first object (which is the module)
> + if i>0:
> + # we must be in an instance, nothing else
> + # we have to check for old-style instances AND
> + # new-style instances.
> + # XXX testing for new-style class instance is tricky
> + # see http://groups.google.fr/groups?th=7bab336f2b4f7e03&seekm=107l13c5tti8876%40news.supernews.com
> + if not (isinstance(obj,InstanceType)):
> + req.log_error('Cannot traverse %s in %s because '
> + '%s is not an instance'
> + %(obj_str,req.unparsed_uri,obj),apache.APLOG_WARNING)
> + raise apache.SERVER_RETURN, apache.HTTP_FORBIDDEN
> +
> + # we know it's OK to call getattr
> + # note that getattr can really call some code because