Modifying classes in Commits - possible attack vector for circumventing custom Class constraints / checks
This issue describes a potential attack for a feature not implemented, at this time.
Atomic-server uses the is-a property to check what classes a Resource has. Depending on this, functionality could be altered. for example, the Invite class is used to create new Agents and give them some rights. This means that Classes can be very security-sensitive.
A malicious user may try something similar to this:
- Create a normal resource
- Give it the properties for Invites that relate to granting rights to something they do not have
- Add the Invite class
(See also #182, note that this problem is currently solved)
The thing is, functionality of classes can be extended with plugins #73. And we need to have a clear strategy on when this logic is executed, and when checks are performed. Currently, we do this after applying, but before saving a Commit. This means that we check the new representation, and make sure it is valid.
However, this also means that a user may break functionality by removing a Class. Is this bad? Not in the previous example.
But what if some class is Extended, which prevents users from editing / removing some specific property? Let's say we have a Forum feature, and users cannot edit their created_at date, and this is protected with some custom logic for things that have a Thread class. What if the user removes the Thread class, then edits the field, and then adds the Thread class again? That would mean the field is edited, even if it was write-protected by the class.
Possible Solutions
- Don't allow modifying Classes. That's a very serious limitation. I would not be able to change my own User to use some new ontology.
- Only perform checks on the Property level, not on entire resources. This could work for the Invite example mentioned before. We could introduce a
before_edit_propertyhook, which is called with the new value, the old value, the commit, and the resource itself. - Besides checking
before_save, we can also checkbefore_apply. This means that a plugin implementation extending a class may need to check both.