[Zope3-dev] Security: getAdapter(self, ISomething) is bad

[Steve Alexander]

Most calls to getAdapter (or queryAdapter) are made on a 
content-object's behalf.

   adapter = getAdapter(obj, ISomething)

In this case, 'obj' is usually security-proxied and context-wrapped.

However, if I write a content object that calls getAdapter for itself, 
the 'self' reference to itself will be entirely unwrapped.
(Unless the method is a ContextMethod or called via a ContextProperty, 
or the object's class derives from ContextAware, in which case the 
'self' argument will be context-wrapped but not security-proxied.)

So, the adapter will be passed an unwrapped object as its context.
A malicious adapter implementation can access and set attributes on this 
object that would normally not be allowed.

Marius and I have altered Step 7 of the Contact example to demonstrate this.

RCS file: 
/cvs-repository/Docs/ZopeComponentArchitecture/PythonProgrammerTutorial/Chapter1/Step7/contact.py,v
retrieving revision 1.2
diff -u -r1.2 contact.py
--- src/zopeproducts/contact/contact.py 7 Apr 2003 10:53:17 -0000       1.2
+++ src/zopeproducts/contact/contact.py 7 Apr 2003 10:54:12 -0000
@@ -4,7 +4,7 @@
  from zope.app.interfaces.container import IAdding
  from zope.app.event import publish
  from zope.app.event.objectevent import ObjectCreatedEvent
-from zope.component import getUtility
+from zope.component import getUtility, getAdapter

  class Contact (persistence.Persistent):
      """Personal contact information
@@ -22,6 +22,7 @@
          return "%s %s" % (self._first, self._last)

      def first(self):
+        info = getAdapter(self, IPostalInfo)
          return self._first

      def last(self):
@@ -74,6 +75,10 @@
      __used_for__ = IContactInfo

      def __init__(self, contact):
+        if type(contact) is Contact:
+            # Muhahaha. Evil adapter has access to a naked contact.
+            contact._first = "I STOLE YOUR IDENTITY"
+            contact._last = "YOU ARE 0WN3D"
          self._contact = contact
          lookup = getUtility(contact, IPostalLookup)
          info = lookup.lookup(contact.postal_code())


In this example, someone has (for benign but, in this case, not 
immediately useful reasons) added a 'getAdapter(self, IPostalInfo)' call 
to the 'first' method of Contact.

Someone else has installed a malicious IPostalInfo adapter that looks 
for an unproxied context, and sets some attributes that it should not be 
allowed to.


It is unclear how best to solve this.
Perhaps getAdapter and queryAdapter should either raise an error if you 
try to adapt something that is not security-proxied. (This will, of 
course, break unit tests...)
Or, perhaps getAdapter and queryAdapter should add a security-proxy 
around the object before it is passed to the adapter factory.
In either case, this adds a dependency between the component 
architecture package and the security proxy package.

There is only one place in the Zope 3 source (at present) that adapts 
from self: the _decode method of BrowserRequest in zope/publisher/browser.py

     def _decode(self, text):
         """Try to decode the text using one of the available
         charsets.
         """
         if self.charsets is None:
             envadaptor = getAdapter(self, IUserPreferredCharsets)
             self.charsets = envadaptor.getPreferredCharsets()
         for charset in self.charsets:
             try:
                 text = unicode(text, charset)
                 break
             except UnicodeError:
                 pass
         return text

In this case, the IUserPerferredCharsets adapter cannot be overridden 
locally because no local context is given. It is not clear how to handle 
local vs global adapters in the case of the request.


--
Steve Alexander