Fix security vulnerability in admin modules

- The Module\BaseAdmin::post method checked credentials but didn't abort the process when it failed
- Created Module\BaseAdmin::checkAdminAccess method

diff --git a/src/Module/Admin/Addons/Details.php b/src/Module/Admin/Addons/Details.php
index 85b17130c4ce1160ccefc758eeafc436bbae122a..4c1fe2df9236f27099e03a72e6f6f23ba5846004 100644 (file)
--- a/src/Module/Admin/Addons/Details.php
+++ b/src/Module/Admin/Addons/Details.php
@@ -32,7 +32,7 @@ class Details extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                $addon = Strings::sanitizeFilePathItem($parameters['addon']);
 

diff --git a/src/Module/Admin/Blocklist/Contact.php b/src/Module/Admin/Blocklist/Contact.php
index 5a7d138b23e06c3ee021187e49de2fcaf3a0bc87..c4eedc5a8a6ee5ace30f809b424f526e96f838fe 100644 (file)
--- a/src/Module/Admin/Blocklist/Contact.php
+++ b/src/Module/Admin/Blocklist/Contact.php
@@ -32,7 +32,7 @@ class Contact extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                self::checkFormSecurityTokenRedirectOnError('/admin/blocklist/contact', 'admin_contactblock');
 

diff --git a/src/Module/Admin/Blocklist/Server.php b/src/Module/Admin/Blocklist/Server.php
index b4be591e7ffa7e4f6dc32651c6bff01a66263795..1290662f25bcb042ebacd9e5b0510bfd052bd2f0 100644 (file)
--- a/src/Module/Admin/Blocklist/Server.php
+++ b/src/Module/Admin/Blocklist/Server.php
@@ -30,7 +30,7 @@ class Server extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                if (empty($_POST['page_blocklist_save']) && empty($_POST['page_blocklist_edit'])) {
                        return;

diff --git a/src/Module/Admin/Features.php b/src/Module/Admin/Features.php
index 51ba9140ef144a00fe081f02747b19c39c705c11..5054da3fb4fef39686958bf52c90643363df7c0a 100644 (file)
--- a/src/Module/Admin/Features.php
+++ b/src/Module/Admin/Features.php
@@ -30,7 +30,7 @@ class Features extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                self::checkFormSecurityTokenRedirectOnError('/admin/features', 'admin_manage_features');
 

diff --git a/src/Module/Admin/Item/Delete.php b/src/Module/Admin/Item/Delete.php
index 028e228d34f63519f318c70d6b1303c24184d97a..9e2bc90d9218d608c3390ec82eab4a1ee8ca8ffb 100644 (file)
--- a/src/Module/Admin/Item/Delete.php
+++ b/src/Module/Admin/Item/Delete.php
@@ -31,7 +31,7 @@ class Delete extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                if (empty($_POST['page_deleteitem_submit'])) {
                        return;

diff --git a/src/Module/Admin/Logs/Settings.php b/src/Module/Admin/Logs/Settings.php
index 0b5993798657c929fcdf9af70089bfd818e6fbe3..7730b487da24d90d81ad1a7f70c599eec7154c68 100644 (file)
--- a/src/Module/Admin/Logs/Settings.php
+++ b/src/Module/Admin/Logs/Settings.php
@@ -31,7 +31,7 @@ class Settings extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                if (empty($_POST['page_logs'])) {
                        return;

diff --git a/src/Module/Admin/PhpInfo.php b/src/Module/Admin/PhpInfo.php
index f282e100891368c3d0acf9b5b33b84506830352c..61a004618e0e8e579c619cf3def0725642fa8271 100644 (file)
--- a/src/Module/Admin/PhpInfo.php
+++ b/src/Module/Admin/PhpInfo.php
@@ -27,7 +27,7 @@ class PhpInfo extends BaseAdmin
 {
        public static function rawContent(array $parameters = [])
        {
-               parent::rawContent($parameters);
+               self::checkAdminAccess();
 
                phpinfo();
                exit();

diff --git a/src/Module/Admin/Site.php b/src/Module/Admin/Site.php
index 6380f3d935bd2658d06670cbf961f9d38c3ae0c2..9f3905da5b1d13d38142077e70143f494bc92412 100644 (file)
--- a/src/Module/Admin/Site.php
+++ b/src/Module/Admin/Site.php
@@ -43,7 +43,7 @@ class Site extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                self::checkFormSecurityTokenRedirectOnError('/admin/site', 'admin_site');
 

diff --git a/src/Module/Admin/Themes/Embed.php b/src/Module/Admin/Themes/Embed.php
index 71824c6ccb64941ad6c0dafa7aedbd87a84f6cfd..a308b43cb52d727170aa6f60471194d9a809371a 100644 (file)
--- a/src/Module/Admin/Themes/Embed.php
+++ b/src/Module/Admin/Themes/Embed.php
@@ -38,7 +38,7 @@ class Embed extends BaseAdmin
 
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                $theme = Strings::sanitizeFilePathItem($parameters['theme']);
                if (is_file("view/theme/$theme/config.php")) {

diff --git a/src/Module/Admin/Tos.php b/src/Module/Admin/Tos.php
index fef199c351fac87b111db9f568b80016bb5ab1bf..aac81264b7e12de28c34afb2e62bdddacf6c40d0 100644 (file)
--- a/src/Module/Admin/Tos.php
+++ b/src/Module/Admin/Tos.php
@@ -29,7 +29,7 @@ class Tos extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                if (empty($_POST['page_tos'])) {
                        return;

diff --git a/src/Module/Admin/Users.php b/src/Module/Admin/Users.php
index 751b618afc3220ddea822867531f913d4d5d103c..74fee7230acd3218c583625b4bac5aa5679fd441 100644 (file)
--- a/src/Module/Admin/Users.php
+++ b/src/Module/Admin/Users.php
@@ -34,7 +34,7 @@ class Users extends BaseAdmin
 {
        public static function post(array $parameters = [])
        {
-               parent::post($parameters);
+               self::checkAdminAccess();
 
                self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users');
 

diff --git a/src/Module/BaseAdmin.php b/src/Module/BaseAdmin.php
index a7b38a50330bac94c2505927ff4f167b68d5eb76..67de97f8577f0978a8f38447f482c4f5abc1295b 100644 (file)
--- a/src/Module/BaseAdmin.php
+++ b/src/Module/BaseAdmin.php
@@ -26,7 +26,7 @@ use Friendica\Core\Addon;
 use Friendica\Core\Renderer;
 use Friendica\Core\Session;
 use Friendica\DI;
-use Friendica\Network\HTTPException\ForbiddenException;
+use Friendica\Network\HTTPException;
 
 require_once 'boot.php';
 
@@ -42,42 +42,35 @@ require_once 'boot.php';
  */
 abstract class BaseAdmin extends BaseModule
 {
-       public static function post(array $parameters = [])
+       /**
+        * @param bool $interactive
+        * @throws HTTPException\ForbiddenException
+        * @throws HTTPException\InternalServerErrorException
+        */
+       public static function checkAdminAccess(bool $interactive = false)
        {
-               if (!is_site_admin()) {
-                       return;
+               if (!local_user()) {
+                       if ($interactive) {
+                               notice(DI::l10n()->t('Please login to continue.'));
+                               Session::set('return_path', DI::args()->getQueryString());
+                               DI::baseUrl()->redirect('login');
+                       } else {
+                               throw new HTTPException\UnauthorizedException(DI::l10n()->t('Please login to continue.'));
+                       }
                }
 
-               // do not allow a page manager to access the admin panel at all.
-               if (!empty($_SESSION['submanage'])) {
-                       return;
-               }
-       }
-
-       public static function rawContent(array $parameters = [])
-       {
                if (!is_site_admin()) {
-                       return '';
+                       throw new HTTPException\ForbiddenException(DI::l10n()->t('You don\'t have access to administration pages.'));
                }
 
                if (!empty($_SESSION['submanage'])) {
-                       return '';
+                       throw new HTTPException\ForbiddenException(DI::l10n()->t('Submanaged account can\'t access the administation pages. Please log back in as the main account.'));
                }
-
-               return '';
        }
 
        public static function content(array $parameters = [])
        {
-               if (!is_site_admin()) {
-                       notice(DI::l10n()->t('Please login to continue.'));
-                       Session::set('return_path', DI::args()->getQueryString());
-                       DI::baseUrl()->redirect('login');
-               }
-
-               if (!empty($_SESSION['submanage'])) {
-                       throw new ForbiddenException(DI::l10n()->t('Submanaged account can\'t access the administation pages. Please log back in as the main account.'));
-               }
+               self::checkAdminAccess(true);
 
                // Header stuff
                DI::page()['htmlhead'] .= Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/settings_head.tpl'), []);