CVE-2021-3980: Exposure of Private Personal Information to an Unauthorized Actor (CWE-359)
value: elgg is vulnerable to Exposure of Private Personal Information to an Unauthorized Actor
CWE description: The product does not properly prevent a person's private, personal information from being accessed by actors who either (1) are not explicitly authorized to access the information or (2) do not have the implicit consent of the person about whom the information is collected.
CVSS v3 base score: 7.5
Published: 2021-12-03T15:15Z
Repository: https://github.com/elgg/elgg
Language: PHP
Files changed: engine/lib/elgglib.php, engine/tests/phpunit/integration/Elgg/Ajax/ControllerIntegrationTest.php, engine/tests/test_files/views/default/admin/ajax_test/registered.php, engine/tests/test_files/views/default/ajax_test/not_registered.php, engine/tests/test_files/views/default/ajax_test/registered.php, engine/tests/test_files/views/default/forms/admin/ajax_test/registered.php, engine/tests/test_files/views/default/forms/ajax_test/not_registered.php, engine/tests/test_files/views/default/forms/ajax_test/registered.php, views/default/forms/admin/user/change_email.php
Commit date: 2021-11-22 12:38:25 +0100
Commit message: fix(ajax): forms in the admin namespace are protected

All forms in the view namespace 'forms/admin' are now protected by an
AdminGatekeeper in case of AJAX requests to 'ajax/form/admin/'. This is
inline with the requests to 'ajax/view/admin'.
Diff: diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php
index 0e95fc1d4d..d8cbe9cce6 100644
--- a/engine/lib/elgglib.php
+++ b/engine/lib/elgglib.php
@@ -1103,10 +1103,14 @@ function _elgg_ajax_page_handler($segments) {
 				elgg_admin_gatekeeper();
 			}
 			// ignore 'view/'
 			$view = implode('/', array_slice($segments, 1));
 		} else {
+			if ($segments[1] === 'admin') {
+				// protect admin forms similar to all admin pages that are protected automatically in the admin_page_handler
+				elgg_admin_gatekeeper();
+			}
 			// form views start with "forms", not "form"
 			$view = 'forms/' . implode('/', array_slice($segments, 1));
 		}
 
 		$ajax_api = _elgg_services()->ajax;
diff --git a/engine/tests/phpunit/integration/Elgg/Ajax/ControllerIntegrationTest.php b/engine/tests/phpunit/integration/Elgg/Ajax/ControllerIntegrationTest.php
new file mode 100644
index 0000000000..272c198eb3
--- /dev/null
+++ b/engine/tests/phpunit/integration/Elgg/Ajax/ControllerIntegrationTest.php
@@ -0,0 +1,164 @@
+<?php
+
+namespace Elgg\Ajax;
+
+use Elgg\IntegrationTestCase;
+
+class ControllerIntegrationTest extends IntegrationTestCase {
+
+	/**
+	 * @var \ElggUser
+	 */
+	protected $user;
+	
+	public function up() {
+		$this->registerViews();
+	}
+
+	public function down() {
+		elgg_get_session()->removeLoggedInUser();
+		
+		if (isset($this->user)) {
+			elgg_call(ELGG_IGNORE_ACCESS, function() {
+				$this->user->delete();
+			});
+		}
+	}
+	
+	protected function prepareService(\Elgg\Http\Request $request) {
+		$this->createApplication([
+			'isolate' => true,
+			'request' => $request,
+		]);
+		
+		$this->registerViews();
+	}
+	
+	protected function executeRequest(\Elgg\Http\Request $request, bool $prepare = true) {
+		if ($prepare) {
+			$this->prepareService($request);
+		}
+		
+		return _elgg_services()->router->getResponse($request);
+	}
+	
+	protected function registerViews() {
+		$views_dir = $this->normalizeTestFilePath('views');
+		_elgg_services()->views->autoregisterViews('', "{$views_dir}/default", 'default');
+		
+		elgg_register_ajax_view('ajax_test/registered');
+		elgg_register_ajax_view('admin/ajax_test/registered');
+		elgg_register_ajax_view('forms/ajax_test/registered');
+		elgg_register_ajax_view('forms/admin/ajax_test/registered');
+	}
+	
+	public function testCanLoadRegisteredAjaxView() {
+		$request = $this->prepareHttpRequest('ajax/view/ajax_test/registered', 'GET', [], 1);
+		
+		$response = $this->executeRequest($request);
+		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
+		$this->assertEquals('registered', $response->getContent());
+	}
+	
+	public function testCantLoadRegisteredAjaxViewWithNonAjaxRequest() {
+		$request = $this->prepareHttpRequest('ajax/view/ajax_test/registered');
+		
+		$this->expectException(\Elgg\Http\Exception\AjaxGatekeeperException::class);
+		$this->executeRequest($request);
+	}
+	
+	public function testCantLoadNonRegisteredAjaxView() {
+		$this->assertTrue(elgg_view_exists('ajax_test/not_registered'));
+		$request = $this->prepareHttpRequest('ajax/view/ajax_test/not_registered', 'GET', [], 1);
+		
+		$response = $this->executeRequest($request);
+		$this->assertInstanceOf(\Elgg\Http\ErrorResponse::class, $response);
+	}
+	
+	public function testCantAccessAdminAjaxViewLoggedOut() {
+		$request = $this->prepareHttpRequest('ajax/view/admin/ajax_test/registered', 'GET', [], 1);
+		
+		$this->expectException(\Elgg\Http\Exception\LoggedInGatekeeperException::class);
+		$this->executeRequest($request);
+	}
+	
+	public function testCantAccessAdminAjaxViewAsNonAdmin() {
+		$request = $this->prepareHttpRequest('ajax/view/admin/ajax_test/registered', 'GET', [], 1);
+		
+		$this->prepareService($request);
+		
+		$this->user = $user = $this->createUser();
+		elgg_get_session()->setLoggedInUser($user);
+		
+		$this->expectException(\Elgg\Http\Exception\AdminGatekeeperException::class);
+		$this->executeRequest($request, false);
+	}
+	
+	public function testCanAccessAdminAjaxViewAsAdmin() {
+		$request = $this->prepareHttpRequest('ajax/view/admin/ajax_test/registered', 'GET', [], 1);
+		
+		$this->prepareService($request);
+		
+		$user = $this->getAdmin();
+		elgg_get_session()->setLoggedInUser($user);
+		
+		$response = $this->executeRequest($request, false);
+		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
+		$this->assertEquals('registered', $response->getContent());
+	}
+	
+	public function testCanLoadRegisteredAjaxForm() {
+		$request = $this->prepareHttpRequest('ajax/form/ajax_test/registered', 'GET', [], 1);
+		
+		$response = $this->executeRequest($request);
+		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
+		$this->assertStringContainsString('registered', $response->getContent());
+	}
+	
+	public function testCantLoadRegisteredAjaxFormWithNonAjaxRequest() {
+		$request = $this->prepareHttpRequest('ajax/form/ajax_test/registered');
+		
+		$this->expectException(\Elgg\Http\Exception\AjaxGatekeeperException::class);
+		$this->executeRequest($request);
+	}
+	
+	public function testCantLoadNonRegisteredAjaxForm() {
+		$this->assertTrue(elgg_view_exists('ajax_test/not_registered'));
+		$request = $this->prepareHttpRequest('ajax/form/ajax_test/not_registered', 'GET', [], 1);
+		
+		$response = $this->executeRequest($request);
+		$this->assertInstanceOf(\Elgg\Http\ErrorResponse::class, $response);
+	}
+	
+	public function testCantAccessAdminAjaxFormLoggedOut() {
+		$request = $this->prepareHttpRequest('ajax/form/admin/ajax_test/registered', 'GET', [], 1);
+		
+		$this->expectException(\Elgg\Http\Exception\LoggedInGatekeeperException::class);
+		$this->executeRequest($request);
+	}
+	
+	public function testCantAccessAdminAjaxFormAsNonAdmin() {
+		$request = $this->prepareHttpRequest('ajax/form/admin/ajax_test/registered', 'GET', [], 1);
+		
+		$this->prepareService($request);
+		
+		$this->user = $user = $this->createUser();
+		elgg_get_session()->setLoggedInUser($user);
+		
+		$this->expectException(\Elgg\Http\Exception\AdminGatekeeperException::class);
+		$this->executeRequest($request, false);
+	}
+	
+	public function testCanAccessAdminAjaxFormAsAdmin() {
+		$request = $this->prepareHttpRequest('ajax/form/admin/ajax_test/registered', 'GET', [], 1);
+		
+		$this->prepareService($request);
+		
+		$user = $this->getAdmin();
+		elgg_get_session()->setLoggedInUser($user);
+		
+		$response = $this->executeRequest($request, false);
+		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
+		$this->assertStringContainsString('registered', $response->getContent());
+	}
+}
diff --git a/engine/tests/test_files/views/default/admin/ajax_test/registered.php b/engine/tests/test_files/views/default/admin/ajax_test/registered.php
new file mode 100644
index 0000000000..d2946cc407
--- /dev/null
+++ b/engine/tests/test_files/views/default/admin/ajax_test/registered.php
@@ -0,0 +1,6 @@
+<?php
+/**
+ * Registered ajax view
+ */
+
+echo 'registered';
diff --git a/engine/tests/test_files/views/default/ajax_test/not_registered.php b/engine/tests/test_files/views/default/ajax_test/not_registered.php
new file mode 100644
index 0000000000..0d63f870dc
--- /dev/null
+++ b/engine/tests/test_files/views/default/ajax_test/not_registered.php
@@ -0,0 +1,6 @@
+<?php
+/**
+ * Non registered ajax view
+ */
+
+echo 'not registered';
diff --git a/engine/tests/test_files/views/default/ajax_test/registered.php b/engine/tests/test_files/views/default/ajax_test/registered.php
new file mode 100644
index 0000000000..d2946cc407
--- /dev/null
+++ b/engine/tests/test_files/views/default/ajax_test/registered.php
@@ -0,0 +1,6 @@
+<?php
+/**
+ * Registered ajax view
+ */
+
+echo 'registered';
diff --git a/engine/tests/test_files/views/default/forms/admin/ajax_test/registered.php b/engine/tests/test_files/views/default/forms/admin/ajax_test/registered.php
new file mode 100644
index 0000000000..d2946cc407
--- /dev/null
+++ b/engine/tests/test_files/views/default/forms/admin/ajax_test/registered.php
@@ -0,0 +1,6 @@
+<?php
+/**
+ * Registered ajax view
+ */
+
+echo 'registered';
diff --git a/engine/tests/test_files/views/default/forms/ajax_test/not_registered.php b/engine/tests/test_files/views/default/forms/ajax_test/not_registered.php
new file mode 100644
index 0000000000..0d63f870dc
--- /dev/null
+++ b/engine/tests/test_files/views/default/forms/ajax_test/not_registered.php
@@ -0,0 +1,6 @@
+<?php
+/**
+ * Non registered ajax view
+ */
+
+echo 'not registered';
diff --git a/engine/tests/test_files/views/default/forms/ajax_test/registered.php b/engine/tests/test_files/views/default/forms/ajax_test/registered.php
new file mode 100644
index 0000000000..d2946cc407
--- /dev/null
+++ b/engine/tests/test_files/views/default/forms/ajax_test/registered.php
@@ -0,0 +1,6 @@
+<?php
+/**
+ * Registered ajax view
+ */
+
+echo 'registered';
diff --git a/views/default/forms/admin/user/change_email.php b/views/default/forms/admin/user/change_email.php
index 1319b4b15e..2b23322594 100644
--- a/views/default/forms/admin/user/change_email.php
+++ b/views/default/forms/admin/user/change_email.php
@@ -8,11 +8,11 @@
 $guid = elgg_extract('user_guid', $vars);
 $user = elgg_call(ELGG_SHOW_DISABLED_ENTITIES, function() use ($guid) {
 	return get_user($guid);
 });
 
-if (!$user instanceof ElggUser) {
+if (!$user instanceof ElggUser || !$user->canEdit()) {
 	return;
 }
 
 echo elgg_view_title(elgg_echo('admin:users:unvalidated:change_email:user', [$user->getDisplayName()]));
 

Vulnerable code: if (!$user instanceof ElggUser) {
Fixed code: 			if ($segments[1] === 'admin') {
				// protect admin forms similar to all admin pages that are protected automatically in the admin_page_handler
				elgg_admin_gatekeeper();
			}
<?php

namespace Elgg\Ajax;

use Elgg\IntegrationTestCase;

class ControllerIntegrationTest extends IntegrationTestCase {

	/**
	 * @var \ElggUser
	 */
	protected $user;
	
	public function up() {
		$this->registerViews();
	}

	public function down() {
		elgg_get_session()->removeLoggedInUser();
		
		if (isset($this->user)) {
			elgg_call(ELGG_IGNORE_ACCESS, function() {
				$this->user->delete();
			});
		}
	}
	
	protected function prepareService(\Elgg\Http\Request $request) {
		$this->createApplication([
			'isolate' => true,
			'request' => $request,
		]);
		
		$this->registerViews();
	}
	
	protected function executeRequest(\Elgg\Http\Request $request, bool $prepare = true) {
		if ($prepare) {
			$this->prepareService($request);
		}
		
		return _elgg_services()->router->getResponse($request);
	}
	
	protected function registerViews() {
		$views_dir = $this->normalizeTestFilePath('views');
		_elgg_services()->views->autoregisterViews('', "{$views_dir}/default", 'default');
		
		elgg_register_ajax_view('ajax_test/registered');
		elgg_register_ajax_view('admin/ajax_test/registered');
		elgg_register_ajax_view('forms/ajax_test/registered');
		elgg_register_ajax_view('forms/admin/ajax_test/registered');
	}
	
	public function testCanLoadRegisteredAjaxView() {
		$request = $this->prepareHttpRequest('ajax/view/ajax_test/registered', 'GET', [], 1);
		
		$response = $this->executeRequest($request);
		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
		$this->assertEquals('registered', $response->getContent());
	}
	
	public function testCantLoadRegisteredAjaxViewWithNonAjaxRequest() {
		$request = $this->prepareHttpRequest('ajax/view/ajax_test/registered');
		
		$this->expectException(\Elgg\Http\Exception\AjaxGatekeeperException::class);
		$this->executeRequest($request);
	}
	
	public function testCantLoadNonRegisteredAjaxView() {
		$this->assertTrue(elgg_view_exists('ajax_test/not_registered'));
		$request = $this->prepareHttpRequest('ajax/view/ajax_test/not_registered', 'GET', [], 1);
		
		$response = $this->executeRequest($request);
		$this->assertInstanceOf(\Elgg\Http\ErrorResponse::class, $response);
	}
	
	public function testCantAccessAdminAjaxViewLoggedOut() {
		$request = $this->prepareHttpRequest('ajax/view/admin/ajax_test/registered', 'GET', [], 1);
		
		$this->expectException(\Elgg\Http\Exception\LoggedInGatekeeperException::class);
		$this->executeRequest($request);
	}
	
	public function testCantAccessAdminAjaxViewAsNonAdmin() {
		$request = $this->prepareHttpRequest('ajax/view/admin/ajax_test/registered', 'GET', [], 1);
		
		$this->prepareService($request);
		
		$this->user = $user = $this->createUser();
		elgg_get_session()->setLoggedInUser($user);
		
		$this->expectException(\Elgg\Http\Exception\AdminGatekeeperException::class);
		$this->executeRequest($request, false);
	}
	
	public function testCanAccessAdminAjaxViewAsAdmin() {
		$request = $this->prepareHttpRequest('ajax/view/admin/ajax_test/registered', 'GET', [], 1);
		
		$this->prepareService($request);
		
		$user = $this->getAdmin();
		elgg_get_session()->setLoggedInUser($user);
		
		$response = $this->executeRequest($request, false);
		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
		$this->assertEquals('registered', $response->getContent());
	}
	
	public function testCanLoadRegisteredAjaxForm() {
		$request = $this->prepareHttpRequest('ajax/form/ajax_test/registered', 'GET', [], 1);
		
		$response = $this->executeRequest($request);
		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
		$this->assertStringContainsString('registered', $response->getContent());
	}
	
	public function testCantLoadRegisteredAjaxFormWithNonAjaxRequest() {
		$request = $this->prepareHttpRequest('ajax/form/ajax_test/registered');
		
		$this->expectException(\Elgg\Http\Exception\AjaxGatekeeperException::class);
		$this->executeRequest($request);
	}
	
	public function testCantLoadNonRegisteredAjaxForm() {
		$this->assertTrue(elgg_view_exists('ajax_test/not_registered'));
		$request = $this->prepareHttpRequest('ajax/form/ajax_test/not_registered', 'GET', [], 1);
		
		$response = $this->executeRequest($request);
		$this->assertInstanceOf(\Elgg\Http\ErrorResponse::class, $response);
	}
	
	public function testCantAccessAdminAjaxFormLoggedOut() {
		$request = $this->prepareHttpRequest('ajax/form/admin/ajax_test/registered', 'GET', [], 1);
		
		$this->expectException(\Elgg\Http\Exception\LoggedInGatekeeperException::class);
		$this->executeRequest($request);
	}
	
	public function testCantAccessAdminAjaxFormAsNonAdmin() {
		$request = $this->prepareHttpRequest('ajax/form/admin/ajax_test/registered', 'GET', [], 1);
		
		$this->prepareService($request);
		
		$this->user = $user = $this->createUser();
		elgg_get_session()->setLoggedInUser($user);
		
		$this->expectException(\Elgg\Http\Exception\AdminGatekeeperException::class);
		$this->executeRequest($request, false);
	}
	
	public function testCanAccessAdminAjaxFormAsAdmin() {
		$request = $this->prepareHttpRequest('ajax/form/admin/ajax_test/registered', 'GET', [], 1);
		
		$this->prepareService($request);
		
		$user = $this->getAdmin();
		elgg_get_session()->setLoggedInUser($user);
		
		$response = $this->executeRequest($request, false);
		$this->assertInstanceOf(\Elgg\Http\OkResponse::class, $response);
		$this->assertStringContainsString('registered', $response->getContent());
	}
}
<?php
/**
 * Registered ajax view
 */

echo 'registered';
<?php
/**
 * Non registered ajax view
 */

echo 'not registered';
<?php
/**
 * Registered ajax view
 */

echo 'registered';
<?php
/**
 * Registered ajax view
 */

echo 'registered';
<?php
/**
 * Non registered ajax view
 */

echo 'not registered';
<?php
/**
 * Registered ajax view
 */

echo 'registered';
if (!$user instanceof ElggUser || !$user->canEdit()) {