fix security issue introduced by 4967:04543ed0bbdc: attributes explicitly set by hooks should not be checked by security hooks
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Mon, 22 Mar 2010 17:58:03 +0100
changeset 4970 1f3d8946ea84
parent 4967 236f1fde6dd0
child 4983 5594aadb740e
fix security issue introduced by 4967:04543ed0bbdc: attributes explicitly set by hooks should not be checked by security hooks
entity.py
hooks/security.py
server/ssplanner.py
--- a/entity.py	Sun Mar 21 18:21:27 2010 +0100
+++ b/entity.py	Mon Mar 22 17:58:03 2010 +0100
@@ -225,6 +225,13 @@
     def __cmp__(self, other):
         raise NotImplementedError('comparison not implemented for %s' % self.__class__)
 
+    def __getitem__(self, key):
+        if key == 'eid':
+            warn('[3.7] entity["eid"] is deprecated, use entity.eid instead',
+                 DeprecationWarning, stacklevel=2)
+            return self.eid
+        return super(Entity, self).__getitem__(key)
+
     def __setitem__(self, attr, value):
         """override __setitem__ to update self.edited_attributes.
 
@@ -242,19 +249,22 @@
             super(Entity, self).__setitem__(attr, value)
             if hasattr(self, 'edited_attributes'):
                 self.edited_attributes.add(attr)
+                self.skip_security_attributes.add(attr)
 
-    def __getitem__(self, key):
-        if key == 'eid':
-            warn('[3.7] entity["eid"] is deprecated, use entity.eid instead',
-                 DeprecationWarning, stacklevel=2)
-            return self.eid
-        return super(Entity, self).__getitem__(key)
+    def setdefault(self, attr, default):
+        """override setdefault to update self.edited_attributes"""
+        super(Entity, self).setdefault(attr, default)
+        if hasattr(self, 'edited_attributes'):
+            self.edited_attributes.add(attr)
+            self.skip_security_attributes.add(attr)
 
-    def setdefault(self, key, default):
-        """override setdefault to update self.edited_attributes"""
-        super(Entity, self).setdefault(key, default)
-        if hasattr(self, 'edited_attributes'):
-            self.edited_attributes.add(key)
+    def rql_set_value(self, attr, value):
+        """call by rql execution plan when some attribute is modified
+
+        don't use dict api in such case since we don't want attribute to be
+        added to skip_security_attributes.
+        """
+        super(Entity, self).__setitem__(attr, value)
 
     def pre_add_hook(self):
         """hook called by the repository before doing anything to add the entity
@@ -867,13 +877,19 @@
 
     # server side utilities ###################################################
 
+    @property
+    def skip_security_attributes(self):
+        try:
+            return self._skip_security_attributes
+        except:
+            self._skip_security_attributes = set()
+            return self._skip_security_attributes
+
     def set_defaults(self):
         """set default values according to the schema"""
-        self._default_set = set()
         for attr, value in self.e_schema.defaults():
             if not self.has_key(attr):
                 self[str(attr)] = value
-                self._default_set.add(attr)
 
     def check(self, creation=False):
         """check this entity against its schema. Only final relation
--- a/hooks/security.py	Sun Mar 21 18:21:27 2010 +0100
+++ b/hooks/security.py	Mon Mar 22 17:58:03 2010 +0100
@@ -16,17 +16,20 @@
 def check_entity_attributes(session, entity, editedattrs=None):
     eid = entity.eid
     eschema = entity.e_schema
-    # ._default_set is only there on entity creation to indicate unspecified
-    # attributes which has been set to a default value defined in the schema
-    defaults = getattr(entity, '_default_set', ())
+    # .skip_security_attributes is there to bypass security for attributes
+    # set by hooks by modifying the entity's dictionnary
+    dontcheck = entity.skip_security_attributes
     if editedattrs is None:
         try:
             editedattrs = entity.edited_attributes
         except AttributeError:
-            editedattrs = entity
+            editedattrs = entity # XXX unexpected
     for attr in editedattrs:
-        if attr in defaults:
+        try:
+            dontcheck.remove(attr)
             continue
+        except KeyError:
+            pass
         rdef = eschema.rdef(attr)
         if rdef.final: # non final relation are checked by other hooks
             # add/delete should be equivalent (XXX: unify them into 'update' ?)
--- a/server/ssplanner.py	Sun Mar 21 18:21:27 2010 +0100
+++ b/server/ssplanner.py	Mon Mar 22 17:58:03 2010 +0100
@@ -473,7 +473,7 @@
                     value = row[index]
                     index += 1
                 if rorder == InsertRelationsStep.FINAL:
-                    edef[rtype] = value
+                    edef.rql_set_value(rtype, value)
                 elif rorder == InsertRelationsStep.RELATION:
                     self.plan.add_relation_def( (edef, rtype, value) )
                     edef.querier_pending_relations[(rtype, 'subject')] = value
@@ -564,7 +564,7 @@
                         edef = edefs[eid]
                     except KeyError:
                         edefs[eid] = edef = session.entity_from_eid(eid)
-                    edef[str(rschema)] = rhsval
+                    edef.rql_set_value(str(rschema), rhsval)
                 else:
                     repo.glob_add_relation(session, lhsval, str(rschema), rhsval)
             result[i] = newrow