-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martijn Pieters wrote:
> On 5/16/06, Cliff Ford <[EMAIL PROTECTED]> wrote:
> 
>> So I still wonder if anyone who is ising the REMOTE_USER environment
>> variable is aware of a problem and has a solution.
> 
> 
> Environment-related variables should not be "hackable" from restricted
> code. Please file a report in de Zope Collector:
> 
>  http://www.zope.org/Collectors/Zope
> 
> You'll need to log in (create a Zope.org account if you don't yet have
> one), and make sure you check the 'security related' tickbox.

Please *don't* mark the issue as "security related", as that only serves
to hide it:  the cat is already out of the bag here, anyway, assuming
that the hole is real:

  - The 'form', 'taintedform', 'cookies', and 'environ' attributes of
    an HTTPRequest are simple Python dicts, and can therefore be mutated
    by untrusted code.

  - It would be possible, although painful, to replace them by a
    derived class.  The pain will come from two axes:

    o Performance:  might not be too bad, depending on the
      implementation;  just using 'class MyDict(dict): pass'
      would be enough to prevent untrusted mutation.  We might
      have to add assertions which allowed '__getitem__' and the
      other accessors, however.  We could also use a "harder"
      implementation, which perhaps raised TypeError from all
      mutators.  I'm attaching a sample patch for the 'environ'
      dict.

    o Backward compatibility:  there may be third-party code which
      more-or-less legitimately expects to mutate one or more of
      those dicts.  We would thus probably have to add a zope.conf
      switch for the "hardening", and default it to the *current*
      setting.

Another solution would be to have the third-party code which suffers
from this flaw monkey-patch the HTTPRequest module to supply the needed
hardening.

Cliff, before we chase all this down, can you verify that the script
actually does allow access in your case?  I can imagine a case where the
authentication was already done by the time the script got to run, which
would make this an annoyance, rather than a hole.


Tres.
- --
===================================================================
Tres Seaver          +1 202-558-7113          [EMAIL PROTECTED]
Palladion Software   "Excellence by Design"    http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEad/Q+gerLs4ltQ4RAq6IAKDWRTIvxT9T1LUhVWkztHa0v4RUAgCbBc/b
2qRA0qkE0u46yaNVW4cH1aY=
=5Cnv
-----END PGP SIGNATURE-----
Index: lib/python/ZPublisher/HTTPRequest.py
===================================================================
--- lib/python/ZPublisher/HTTPRequest.py	(revision 68139)
+++ lib/python/ZPublisher/HTTPRequest.py	(working copy)
@@ -63,6 +63,16 @@
 class NestedLoopExit( Exception ):
     pass
 
+class ReadOnlyDict(dict):
+    def __setitem__(self, key, value):
+        raise TypeError, 'Immutable'
+    def __delitem__(self, key):
+        raise TypeError, 'Immutable'
+    def update(self, other):
+        raise TypeError, 'Immutable'
+    def clear(self):
+        raise TypeError, 'Immutable'
+
 class HTTPRequest(BaseRequest):
     """\
     Model HTTP request data.
@@ -252,7 +262,7 @@
             del environ['HTTP_AUTHORIZATION']
 
         self.stdin=stdin
-        self.environ=environ
+        self.environ=ReadOnlyDict(environ)
         have_env=environ.has_key
         get_env=environ.get
         self.response=response
Index: lib/python/ZPublisher/tests/testHTTPRequest.py
===================================================================
--- lib/python/ZPublisher/tests/testHTTPRequest.py	(revision 68139)
+++ lib/python/ZPublisher/tests/testHTTPRequest.py	(working copy)
@@ -684,7 +684,23 @@
         req.close()
         self.assertEqual(start_count, sys.getrefcount(s))  # The test
 
+    def test_environ_is_immutable(self):
+        from StringIO import StringIO
+        s = StringIO(TEST_FILE_DATA)
+        env = TEST_ENVIRON.copy()
+        env['to_replace'] = 'to_replace'
+        env['to_remove'] = 'to_remove'
+        from ZPublisher.HTTPRequest import HTTPRequest
+        req = HTTPRequest(s, env, None)
+        self.assertRaises(TypeError, req.environ.__setitem__,
+                            'hacked', 'hacked')
+        self.assertRaises(TypeError, req.environ.__setitem__,
+                            'to_replace', 'replaced')
+        self.assertRaises(TypeError, req.environ.__delitem__, 'to_remove')
+        self.assertRaises(TypeError, req.environ.update, {'hacked': 'hacked'})
+        self.assertRaises(TypeError, req.environ.clear)
 
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(AuthCredentialsTestsa, 'test'))
Index: lib/python/OFS/tests/testRanges.py
===================================================================
--- lib/python/OFS/tests/testRanges.py	(revision 68139)
+++ lib/python/OFS/tests/testRanges.py	(working copy)
@@ -59,6 +59,9 @@
             r['Application'] = a
             self.root = a
             self.app = makerequest(self.root, stdout=self.responseOut)
+            # 'environ' is now immutable, so replace it to allow scribbling
+            #  in tests
+            self.app.REQUEST.environ = dict(self.app.REQUEST.environ)
             try: self.app._delObject(TESTFOLDER_NAME)
             except AttributeError: pass
             manage_addFolder(self.app, TESTFOLDER_NAME)
_______________________________________________
Zope maillist  -  Zope@zope.org
http://mail.zope.org/mailman/listinfo/zope
**   No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope-dev )

Reply via email to