Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Privilege escalation in @sharing endpoint (PloneHotfix20200121) #857

Closed
mauritsvanrees opened this issue Jan 22, 2020 · 0 comments · Fixed by #859
Closed

Privilege escalation in @sharing endpoint (PloneHotfix20200121) #857

mauritsvanrees opened this issue Jan 22, 2020 · 0 comments · Fixed by #859

Comments

@mauritsvanrees
Copy link
Member

plone.restapi.deserializer.local_roles.DeserializeFromJson has a weakness.
This endpoint was introduced in plone.restapi 1.0a10 (2017-03-22).

Please apply the patch from restapi_local_roles.py from the hotfix. Should be this, but please check:

--- a/PloneHotfixNext/Products/PloneHotfixNext/restapi_local_roles.py
+++ b/PloneHotfixNext/Products/PloneHotfixNext/restapi_local_roles.py
@@ -36,11 +36,17 @@ if DeserializeFromJson is not None:
         # roles
         roles_reindex = False
         new_roles = data.get("entries", None)
+        managed_roles = frozenset([r['id'] for r in sharing_view.roles()])
+
         if new_roles is not None:
             # the roles are converted into a FrozenSet so we have to filter
             # the data structure we get.
             for user in new_roles:
                 roles_list = [key for key in user["roles"] if user["roles"][key]]
+
+                # Limit roles to ones the user is allowed to delegate
+                roles_list = set(roles_list).intersection(managed_roles)
+
                 user["roles"] = roles_list
             roles_reindex = sharing_view.update_role_settings(new_roles, reindex=False)

@lukasgraf since you and Niklaus discovered this, could you take care of this?

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants