CVE-2021-3964: Authorization Bypass Through User-Controlled Key (CWE-639)
value: elgg is vulnerable to Authorization Bypass Through User-Controlled Key
CWE description: The system's authorization functionality does not prevent one user from gaining access to another user's data or record by modifying the key value identifying the data.
CVSS v3 base score: 5.9
Published: 2021-12-01T12:15Z
Repository: https://github.com/elgg/elgg
Language: PHP
Files changed: mod/groups/actions/groups/membership/delete_invite.php, mod/groups/actions/groups/membership/delete_request.php, mod/groups/actions/groups/membership/join.php, mod/groups/actions/groups/membership/leave.php
Commit date: 2021-11-16 10:29:30 +0100
Commit message: fix(groups): prevent misuse of group membership actions

Only allow actions to be executed for authorised requests.
Diff: diff --git a/mod/groups/actions/groups/membership/delete_invite.php b/mod/groups/actions/groups/membership/delete_invite.php
index 29500e9a1..2809486c2 100644
--- a/mod/groups/actions/groups/membership/delete_invite.php
+++ b/mod/groups/actions/groups/membership/delete_invite.php
@@ -11,12 +11,16 @@ $user = get_user($user_guid);
 // invisible groups require overriding access to delete invite
 $group = elgg_call(ELGG_IGNORE_ACCESS, function() use ($group_guid) {
 	return get_entity($group_guid);
 });
 
-if (!$user && !($group instanceof \ElggGroup)) {
-	return elgg_error_response();
+if (!$user && !$group instanceof \ElggGroup) {
+	return elgg_error_response(elgg_echo('error:missing_data'));
+}
+
+if (!$user->canEdit() && !$group->canEdit()) {
+	return elgg_error_response(elgg_echo('actionunauthorized'));
 }
 
 // If join request made
 $message = '';
 if (remove_entity_relationship($group->guid, 'invited', $user->guid)) {
diff --git a/mod/groups/actions/groups/membership/delete_request.php b/mod/groups/actions/groups/membership/delete_request.php
index af5a78176..04efb9630 100644
--- a/mod/groups/actions/groups/membership/delete_request.php
+++ b/mod/groups/actions/groups/membership/delete_request.php
@@ -7,12 +7,16 @@ $user_guid = (int) get_input('user_guid', elgg_get_logged_in_user_guid());
 $group_guid = (int) get_input('group_guid');
 
 $user = get_user($user_guid);
 $group = get_entity($group_guid);
 
-if (!$user && !($group instanceof \ElggGroup)) {
-	return elgg_error_response();
+if (!$user && !$group instanceof \ElggGroup) {
+	return elgg_error_response(elgg_echo('error:missing_data'));
+}
+
+if (!$user->canEdit() && !$group->canEdit()) {
+	return elgg_error_response(elgg_echo('actionunauthorized'));
 }
 
 // If join request made
 $message = '';
 if (remove_entity_relationship($user->guid, 'membership_request', $group->guid)) {
diff --git a/mod/groups/actions/groups/membership/join.php b/mod/groups/actions/groups/membership/join.php
index e9c894503..9d94a9101 100644
--- a/mod/groups/actions/groups/membership/join.php
+++ b/mod/groups/actions/groups/membership/join.php
@@ -16,14 +16,18 @@ $user = get_user($user_guid);
 // access bypass for getting invisible group
 $group = elgg_call(ELGG_IGNORE_ACCESS, function() use ($group_guid) {
 	return get_entity($group_guid);
 });
 
-if (!$user || !($group instanceof \ElggGroup)) {
+if (!$user || !$group instanceof \ElggGroup) {
 	return elgg_error_response(elgg_echo('groups:cantjoin'));
 }
 
+if (!$user->canEdit() && !$group->canEdit()) {
+	return elgg_error_response(elgg_echo('actionunauthorized'));
+}
+
 // join or request
 $join = false;
 if ($group->isPublicMembership() || $group->canEdit($user->guid)) {
 	// anyone can join public groups and admins can join any group
 	$join = true;
diff --git a/mod/groups/actions/groups/membership/leave.php b/mod/groups/actions/groups/membership/leave.php
index bbea6bfde..33cfdcf42 100644
--- a/mod/groups/actions/groups/membership/leave.php
+++ b/mod/groups/actions/groups/membership/leave.php
@@ -7,14 +7,18 @@ $user_guid = (int) get_input('user_guid', elgg_get_logged_in_user_guid());
 $group_guid = (int) get_input('group_guid');
 
 $user = get_user($user_guid);
 $group = get_entity($group_guid);
 
-if (!$user || !($group instanceof \ElggGroup)) {
+if (!$user || !$group instanceof \ElggGroup) {
 	return elgg_error_response(elgg_echo('groups:cantleave'));
 }
 
+if (!$user->canEdit() && !$group->canEdit()) {
+	return elgg_error_response(elgg_echo('actionunauthorized'));
+}
+
 if ($group->getOwnerGUID() === $user->guid) {
 	// owner can't be removed
 	return elgg_error_response(elgg_echo('groups:cantleave'));
 }
 

Vulnerable code: if (!$user && !($group instanceof \ElggGroup)) {
	return elgg_error_response();
if (!$user && !($group instanceof \ElggGroup)) {
	return elgg_error_response();
if (!$user || !($group instanceof \ElggGroup)) {
if (!$user || !($group instanceof \ElggGroup)) {
Fixed code: if (!$user && !$group instanceof \ElggGroup) {
	return elgg_error_response(elgg_echo('error:missing_data'));
}

if (!$user->canEdit() && !$group->canEdit()) {
	return elgg_error_response(elgg_echo('actionunauthorized'));
if (!$user && !$group instanceof \ElggGroup) {
	return elgg_error_response(elgg_echo('error:missing_data'));
}

if (!$user->canEdit() && !$group->canEdit()) {
	return elgg_error_response(elgg_echo('actionunauthorized'));
if (!$user || !$group instanceof \ElggGroup) {
if (!$user->canEdit() && !$group->canEdit()) {
	return elgg_error_response(elgg_echo('actionunauthorized'));
}

if (!$user || !$group instanceof \ElggGroup) {
if (!$user->canEdit() && !$group->canEdit()) {
	return elgg_error_response(elgg_echo('actionunauthorized'));
}