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
This commit is contained in:
Hypolite Petovan 2020-09-08 10:44:27 -04:00
parent 9bc2c5a52e
commit 3efa8648c5
12 changed files with 29 additions and 36 deletions

View file

@ -32,7 +32,7 @@ class Details extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
$addon = Strings::sanitizeFilePathItem($parameters['addon']); $addon = Strings::sanitizeFilePathItem($parameters['addon']);

View file

@ -32,7 +32,7 @@ class Contact extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
self::checkFormSecurityTokenRedirectOnError('/admin/blocklist/contact', 'admin_contactblock'); self::checkFormSecurityTokenRedirectOnError('/admin/blocklist/contact', 'admin_contactblock');

View file

@ -30,7 +30,7 @@ class Server extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
if (empty($_POST['page_blocklist_save']) && empty($_POST['page_blocklist_edit'])) { if (empty($_POST['page_blocklist_save']) && empty($_POST['page_blocklist_edit'])) {
return; return;

View file

@ -30,7 +30,7 @@ class Features extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
self::checkFormSecurityTokenRedirectOnError('/admin/features', 'admin_manage_features'); self::checkFormSecurityTokenRedirectOnError('/admin/features', 'admin_manage_features');

View file

@ -31,7 +31,7 @@ class Delete extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
if (empty($_POST['page_deleteitem_submit'])) { if (empty($_POST['page_deleteitem_submit'])) {
return; return;

View file

@ -31,7 +31,7 @@ class Settings extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
if (empty($_POST['page_logs'])) { if (empty($_POST['page_logs'])) {
return; return;

View file

@ -27,7 +27,7 @@ class PhpInfo extends BaseAdmin
{ {
public static function rawContent(array $parameters = []) public static function rawContent(array $parameters = [])
{ {
parent::rawContent($parameters); self::checkAdminAccess();
phpinfo(); phpinfo();
exit(); exit();

View file

@ -43,7 +43,7 @@ class Site extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
self::checkFormSecurityTokenRedirectOnError('/admin/site', 'admin_site'); self::checkFormSecurityTokenRedirectOnError('/admin/site', 'admin_site');

View file

@ -38,7 +38,7 @@ class Embed extends BaseAdmin
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
$theme = Strings::sanitizeFilePathItem($parameters['theme']); $theme = Strings::sanitizeFilePathItem($parameters['theme']);
if (is_file("view/theme/$theme/config.php")) { if (is_file("view/theme/$theme/config.php")) {

View file

@ -29,7 +29,7 @@ class Tos extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
if (empty($_POST['page_tos'])) { if (empty($_POST['page_tos'])) {
return; return;

View file

@ -34,7 +34,7 @@ class Users extends BaseAdmin
{ {
public static function post(array $parameters = []) public static function post(array $parameters = [])
{ {
parent::post($parameters); self::checkAdminAccess();
self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users'); self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users');

View file

@ -26,7 +26,7 @@ use Friendica\Core\Addon;
use Friendica\Core\Renderer; use Friendica\Core\Renderer;
use Friendica\Core\Session; use Friendica\Core\Session;
use Friendica\DI; use Friendica\DI;
use Friendica\Network\HTTPException\ForbiddenException; use Friendica\Network\HTTPException;
require_once 'boot.php'; require_once 'boot.php';
@ -42,42 +42,35 @@ require_once 'boot.php';
*/ */
abstract class BaseAdmin extends BaseModule 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()) { if (!local_user()) {
return; 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()) { if (!is_site_admin()) {
return ''; throw new HTTPException\ForbiddenException(DI::l10n()->t('You don\'t have access to administration pages.'));
} }
if (!empty($_SESSION['submanage'])) { 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 = []) public static function content(array $parameters = [])
{ {
if (!is_site_admin()) { self::checkAdminAccess(true);
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.'));
}
// Header stuff // Header stuff
DI::page()['htmlhead'] .= Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/settings_head.tpl'), []); DI::page()['htmlhead'] .= Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/settings_head.tpl'), []);